[PATCH] devicetree: dt_sysinfo: Reference compatible whitespace

[Colin Ian King]

On 19/09/16 19:47, Deb McLemore wrote:
> Remove whitespace from the model property when checking the Reference
> compatibility feature.
> 
> Signed-off-by: Deb McLemore <debmc at linux.vnet.ibm.com>
> ---
>  src/devicetree/dt_sysinfo/dt_sysinfo.c | 56 +++++++++++++++++++++++++++++-----
>  src/lib/include/fwts_devicetree.h      |  3 ++
>  src/lib/src/fwts_devicetree.c          | 28 +++++++++++++++++
>  3 files changed, 79 insertions(+), 8 deletions(-)
> 
> diff --git a/src/devicetree/dt_sysinfo/dt_sysinfo.c b/src/devicetree/dt_sysinfo/dt_sysinfo.c
> index 0565979..f485d61 100644
> --- a/src/devicetree/dt_sysinfo/dt_sysinfo.c
> +++ b/src/devicetree/dt_sysinfo/dt_sysinfo.c
> @@ -29,13 +29,13 @@
>  static const char op_powernv[] = "ibm,powernv";
>  
>  static const char *firestone_models[] = {
> -	"8335-GTA        ",
> -	"8335-GCA        ",
> -	"8335-GTX        ",
> +	"8335-GTA",
> +	"8335-GCA",
> +	"8335-GTX",
>  };
>  
>  static const char *garrison_models[] = {
> -	"8335-GTB        ",
> +	"8335-GTB",
>  };
>  
>  static struct reference_platform {
> @@ -186,6 +186,7 @@ static int dt_sysinfo_check_ref_plat_compatible(fwts_framework *fw)
>  		return FWTS_ERROR;
>  	} else {
>  		const char *model_buf, *compat_buf;
> +		char *orig_model_buf, *tmp_model_buf;
>  
>  		compat_buf = fdt_getprop(fw->fdt, node,
>  				"compatible", &compat_len);
> @@ -194,16 +195,54 @@ static int dt_sysinfo_check_ref_plat_compatible(fwts_framework *fw)
>  
>  		if (!model_buf || !compat_buf) {
>  			fwts_failed(fw,LOG_LEVEL_HIGH,
> -				"DTSysinfoPropertyMissing",
> -				"can't read properties for OpenPOWER"
> +				"DTSysInfoCheck:",
> +				" Cannot read the properties for OpenPOWER"
>  				" Reference Compatible check");
>  			return FWTS_ERROR;
>  		}
>  
> +		/* need modifiable memory    */
> +		/* save original ptr to free */
> +		tmp_model_buf = orig_model_buf = strdup(model_buf);
> +		if (!tmp_model_buf) {
> +			fwts_failed(fw, LOG_LEVEL_HIGH,
> +				"DTSysInfoCheck:",
> +				" Unable to get memory for model"
> +				" compare for OpenPOWER"
> +				" Reference Compatible check");
> +			return FWTS_ERROR;
> +		}
> +
> +		tmp_model_buf = hidewhitespace(tmp_model_buf);
> +		if (!(strcmp(model_buf, tmp_model_buf) == 0)) {
> +			fwts_warning(fw,
> +				"DTSysInfoCheck:"
> +				" See further advice in the log.\n");
> +			fwts_log_nl(fw);
> +			fwts_log_info_verbatim(fw,
> +				"DTSysInfoCheck:"
> +				" Check the root \"model\" property"
> +				" from the device tree %s \"%s\".\n",
> +				DT_FS_PATH,
> +				model_buf);
> +			fwts_advice(fw,
> +				"Check the root \"model\" property"
> +				" from the device tree %s, "
> +				"there are whitespace inconsistentencies"
> +				" between the \"model\" property and "
> +				" the trimmed value of \"%s\", report"
> +				" this as a possible bug."
> +				"  Run \"hexdump -C model\""
> +				" from the \"%s\" directory to view"
> +				" the raw contents of the property.",
> +				DT_FS_PATH,
> +				tmp_model_buf,
> +				DT_FS_PATH);
> +		}
>  		if (machine_matches_reference_model(fw,
>  				compat_buf,
>  				compat_len,
> -				model_buf)) {
> +				tmp_model_buf)) {
>  			fwts_passed(fw, "OpenPOWER Reference "
>  				"Compatible passed");
>  		} else {
> @@ -214,9 +253,10 @@ static int dt_sysinfo_check_ref_plat_compatible(fwts_framework *fw)
>  			/* adding verbatim to show proper string */
>  			fwts_log_info_verbatim(fw,
>  			"Unable to find an OpenPOWER reference"
> -			" match for \"%s\"", model_buf);
> +			" match for \"%s\"", tmp_model_buf);
>  			return FWTS_ERROR;

Static analysis found a resource leak on the above return statement.
orig_model_buf needs free'ing before the return.

    CID 1362863:  Resource leaks  (RESOURCE_LEAK)
    Variable "orig_model_buf" going out of scope leaks the storage it
points to.

>  		}
> +		free(orig_model_buf);
>  	}
>  
>  	return FWTS_OK;
> diff --git a/src/lib/include/fwts_devicetree.h b/src/lib/include/fwts_devicetree.h
> index a0ea9d0..df03207 100644
> --- a/src/lib/include/fwts_devicetree.h
> +++ b/src/lib/include/fwts_devicetree.h
> @@ -37,6 +37,7 @@
>  #define DT_PROPERTY_OPAL_MANUFACTURER_ID "manufacturer-id"
>  #define DT_PROPERTY_OPAL_STATUS "status"
>  #define DT_PROPERTY_OPAL_VENDOR "vendor"
> +#define DT_PROPERTY_OPAL_BOARD_INFO "board-info"
>  
>  #if FWTS_HAS_DEVICETREE
>  
> @@ -55,4 +56,6 @@ int check_property_printable(fwts_framework *fw,
>  			const char *buf,
>  			size_t len);
>  
> +char *hidewhitespace(char *name);
> +
>  #endif
> diff --git a/src/lib/src/fwts_devicetree.c b/src/lib/src/fwts_devicetree.c
> index d7facb0..35b5385 100644
> --- a/src/lib/src/fwts_devicetree.c
> +++ b/src/lib/src/fwts_devicetree.c
> @@ -109,5 +109,33 @@ int check_property_printable(fwts_framework *fw,
>  		name,
>  		buf);
>  
> +	fwts_passed(fw,
> +		"DTPrintableProperty \"%s\" passed",
> +		name);
> +
>  	return FWTS_OK;
>  }
> +
> +/* hidewhitespace (char *name)                    */
> +/*                                                */
> +/* Caller must pass in modifiable memory for name */
> +/* Caller must save original memory ptr to free   */
> +/* the original allocated memory if needed        */
> +
> +char *hidewhitespace(char *name)
> +{
> +	char *end;
> +
> +	while (isspace(*name)) name++;

Minor quibbles below, nothing major, but I'd prefer the C style:

	while (isspace(*name))
		name++;

> +
> +	if (*name == 0)

perhaps:
	if (!*name)

> +		return name;
> +
> +	end = name + strlen(name) -1;
> +	while (end > name && isspace(*end)) end--;

and here:
	while ((end > name) && isspace(*end))
		end--;

> +
> +	*(end+1) = 0;

and explicit end-of-string escaped char:

	*(end + 1) = '\0';

> +
> +	return name;
> +
> +}
> 
Thanks Deb

Colin