Re: [dmidecode] [PATCH] dmioem: Fix segmentation fault in dmi_hp_240_attr()

[Jean Delvare]

Hi Jerry,

On Fri, 5 Aug 2022 11:57:04 +0200, Jean Delvare wrote:
> Thinking about it some more, it might make sense to actually extend
> pr_list_item() to be more flexible, by accepting an optional label,
> instead of having two separate functions. I'll give it a try and see
> how it looks.

So I gave a try to this idea. On the positive side, it avoids
duplicating the code. On the negative side, it requires updating 26
calling sites in dmidecode.c, and more importantly, increases the
binary size and execution time as one more (unused) parameter needs to
be passed each time.

So I went back to square one and came up with a more simple, less
intrusive alternative. It's not utterly elegant, but the 2 other
options look even worse to my eyes, so I tend to prefer that one. The
idea is to pack the label and the value into a single formatted string
on the caller side. What do you think?

==========

pr_attr() does not accept a NULL format string. glibc can deal with
it, but FreeBSD's libc chokes on it.

Display the attributes as a list instead. Pack the attribute name and
status into a single formatted string that can be passed to
pr_list_item(). That's arguably a hack, but it's cheap, non-intrusive,
and works nicely in the end.

Bug reported by Scott Benesh (Microchip).

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Fixes: a4b31b2bc537 ("dmioem: Present HPE type 240 attributes in a nicer way")
Cc: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 dmioem.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- dmidecode.orig/dmioem.c     2022-09-07 16:01:38.304638614 +0200
+++ dmidecode/dmioem.c  2022-09-07 16:01:59.546941622 +0200
@@ -198,13 +198,14 @@ static void dmi_hp_240_attr(u64 defined,
        };
        unsigned int i;
 
-       pr_attr("Attributes Defined/Set", NULL);
+       pr_list_start("Attributes Defined/Set", NULL);
        for (i = 0; i < ARRAY_SIZE(attributes); i++)
        {
                if (!(defined.l & (1UL << i)))
                        continue;
-               pr_subattr(attributes[i], "%s", set.l & (1UL << i) ? "Yes" : 
"No");
+               pr_list_item("%s: %s", attributes[i], set.l & (1UL << i) ? 
"Yes" : "No");
        }
+       pr_list_end();
 }
 
 static void dmi_hp_203_assoc_hndl(const char *fname, u16 num)


-- 
Jean Delvare
SUSE L3 Support