ACK: [PATCH 4/4] dmicheck: use dmi_load_file in preference to fwts_mmap

[Colin Ian King]

On 15/11/2018 23:25, Leif Lindholm wrote:
> dmi_load_file uses the /sys interface to access SMBIOS tables, whereas
> fwts_mmap maps (or attempts to map) tables directly through /dev/mem.
> Use the proper kernel interface in preference, falling back to /dev/mem
> where that is not available.
> 
> Signed-off-by: Leif Lindholm <leif.lindholm at linaro.org>
> ---
> 
> Note: this one is really unrelated from the other 3 patches in the
>       series, but it sprung from the same itch, so I included it.
> 
>  src/dmi/dmicheck/dmicheck.c | 47 +++++++++++++++++++++++----------------------
>  1 file changed, 24 insertions(+), 23 deletions(-)
> 
> diff --git a/src/dmi/dmicheck/dmicheck.c b/src/dmi/dmicheck/dmicheck.c
> index c794973b..19c1bd48 100644
> --- a/src/dmi/dmicheck/dmicheck.c
> +++ b/src/dmi/dmicheck/dmicheck.c
> @@ -322,6 +322,18 @@ static void* dmi_table_smbios(fwts_framework *fw, fwts_smbios_entry *entry)
>  		return NULL;
>  	}
>  
> +	if (dmi_load_file("/sys/firmware/dmi/tables/smbios_entry_point", anchor, 4) == FWTS_OK
> +			&& strncmp(anchor, "_SM_", 4) == 0) {
> +		table = malloc(length);
> +		if (!table)
> +			return NULL;
> +		if (dmi_load_file("/sys/firmware/dmi/tables/DMI", table, length) == FWTS_OK) {
> +			fwts_log_info(fw, "SMBIOS table loaded from /sys/firmware/dmi/tables/DMI\n");
> +			return table;
> +		}
> +		free(table);
> +	}
> +
>  	mem = fwts_mmap(addr, length);
>  	if (mem != FWTS_MAP_FAILED) {
>  		/* Can we safely copy the table? */
> @@ -337,17 +349,6 @@ static void* dmi_table_smbios(fwts_framework *fw, fwts_smbios_entry *entry)
>  		return table;
>  	}
>  
> -	if (dmi_load_file("/sys/firmware/dmi/tables/smbios_entry_point", anchor, 4) == FWTS_OK
> -			&& strncmp(anchor, "_SM_", 4) == 0) {
> -		table = malloc(length);
> -		if (!table)
> -			return NULL;
> -		if (dmi_load_file("/sys/firmware/dmi/tables/DMI", table, length) == FWTS_OK) {
> -			fwts_log_info(fw, "SMBIOS table loaded from /sys/firmware/dmi/tables/DMI\n");
> -			return table;
> -		}
> -		free(table);
> -	}
>  
>  	fwts_log_error(fw, "Cannot mmap SMBIOS table from %8.8" PRIx32 "..%8.8" PRIx32 ".",
>  			entry->struct_table_address, entry->struct_table_address + entry->struct_table_length);
> @@ -369,6 +370,18 @@ static void* dmi_table_smbios30(fwts_framework *fw, fwts_smbios30_entry *entry)
>  		return NULL;
>  	}
>  
> +	if (dmi_load_file("/sys/firmware/dmi/tables/smbios_entry_point", anchor, 5) == FWTS_OK
> +			&& strncmp(anchor, "_SM3_", 5) == 0) {
> +		table = malloc(length);
> +		if (!table)
> +			return NULL;
> +		if (dmi_load_file_variable_size("/sys/firmware/dmi/tables/DMI", table, &length) == FWTS_OK) {
> +			fwts_log_info(fw, "SMBIOS30 table loaded from /sys/firmware/dmi/tables/DMI\n");
> +			return table;
> +		}
> +		free(table);
> +	}
> +
>  	mem = fwts_mmap(addr, length);
>  	if (mem != FWTS_MAP_FAILED) {
>  		/* Can we safely copy the table? */
> @@ -384,18 +397,6 @@ static void* dmi_table_smbios30(fwts_framework *fw, fwts_smbios30_entry *entry)
>  		return table;
>  	}
>  
> -	if (dmi_load_file("/sys/firmware/dmi/tables/smbios_entry_point", anchor, 5) == FWTS_OK
> -			&& strncmp(anchor, "_SM3_", 5) == 0) {
> -		table = malloc(length);
> -		if (!table)
> -			return NULL;
> -		if (dmi_load_file_variable_size("/sys/firmware/dmi/tables/DMI", table, &length) == FWTS_OK) {
> -			fwts_log_info(fw, "SMBIOS30 table loaded from /sys/firmware/dmi/tables/DMI\n");
> -			return table;
> -		}
> -		free(table);
> -	}
> -
>  	fwts_log_error(fw, "Cannot mmap SMBIOS 3.0 table from %16.16" PRIx64 "..%16.16" PRIx64 ".",
>  			entry->struct_table_address, entry->struct_table_address + entry->struct_table_max_size);
>  	return NULL;
> 

Acked-by: Colin Ian King <colin.king at canonical.com>

Thanks Leif for these updates, much appreciated.

Colin