[RFC, PATCH 4/7 v2] fwts: Print names of missing features, rather than a cryptic bitmask

Jeremy Kerr jk at ozlabs.org
Thu May 1 10:45:11 UTC 2014


Hi Colin,

>> --- a/src/lib/src/fwts_firmware.c
>> +++ b/src/lib/src/fwts_firmware.c
>> @@ -26,6 +26,13 @@
>>  static enum firmware_type firmware_type;
>>  static bool firmware_type_valid;
>>  
>> +static struct {
>> +	enum firmware_feature feature;
>> +	const char name[16];
>> +} feature_names[] = {
>> +	{ FWTS_FW_FEATURE_ACPI,		"acpi" },
> 
> Most of fwts "acpi" is in capitals, perhaps this should be "ACPI" just
> for consistency

OK, happy to go with whatever the convention is.

>> +};
>> +
>>  /*
>>   *  fwts_memory_map_entry_compare()
>>   *	callback used to sort memory_map entries on start address
>> @@ -64,3 +71,43 @@ int fwts_firmware_features(void)
>>  
>>  	return features;
>>  }
>> +
>> +const char *fwts_firmware_feature_string(int features)
> 
> very minor quibble, perhaps that could be: const int features

I noticed that in a few places. Happy to change it, but curious as to
why you have consts for pass-by-value arguments?

>> +{
>> +	const int n = FWTS_ARRAY_LEN(feature_names);
>> +	const char sep[] = ", ";
>> +	static char str[50];
>> +	int i, len;
> 
> perhaps len could be size_t

Sounds good.

Thanks,


Jeremy



More information about the fwts-devel mailing list