Re: [dmidecode] Adding -s bios-revision & -s firmware-revision support

[Jean Delvare]

Hi Erwan,

On Wed, 2 Oct 2019 16:31:15 +0200, Erwan Velu wrote:
> I'm fine with that but wonder how to handle the case where the value
> is not avail.
> 
> If a do something like printf("\tFirmware Revision: %s\n",
> dmi_revision(data[0x16], data[0x17]));

The above is not really workable, as you would need a temporary buffer,
something we carefully avoid everywhere in dmidecode ;-) dmi_*_revision
functions should print the result themselves, not return a string.

> The output of -q -t 0 will look like : "Firmware Revision: " which sounds 
> odds.

I don't follow you. If the value is not available, you wouldn't print
anything at all, as the code did before your patches. That's what the
h->length checks are for.

> Any suggestions on that ?

The output of "dmidecode -t 0" and "dmidecode -q -t 0" should simply be
unchanged by your patches.

For option -s, don't print anything if the field is not available.
There's already code in dmi_table_string() for that purpose:

        if (offset >= h->length)
                return;

By the way, I just noticed that this check is not reliable in your
case, because you don't just use data[offset] but also data[offset+1].
So you would need an explicit check for "offset+1 >= h->length", or
alternatively, pass the offset of the second byte as a parameter, and
use (data[offset-1], data[offset]) as your dmi_*_revision function
parameters - I admit it's a bit tricky but it avoids the extra,
partially redundant check. Your call.

-- 
Jean Delvare
SUSE L3 Support