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

[Jean Delvare]

On Fri, 5 Aug 2022 11:45:19 +0200, Jean Delvare wrote:
> On Thu, 4 Aug 2022 11:29:05 -0600, Jerry Hoemann wrote:
> > Did you consider adding a check that format is nonzero to pr_attr()
> > like pr_list_start() does?  
> 
> Yes, I considered that first, especially as the other solution
> duplicates code, which isn't appealing. However I decided against it
> for semantic reasons.

Bahhhh, I hit "Send" by accident, sorry.

So the rationale for my decision is that, if there is no value, you
aren't really printing an attribute. That's really only a header for a
list to come, exactly as pr_list_start/pr_list_item/pr_list_end are
meant for. It turns out that so far the lists printed by pr_list_item
were simple (only values) while what you need here is label+value list
items, thus the need to extend the concept.

While it doesn't really make a difference for the current plain text
output (as shown by the code duplication), it will matter if we ever
implement alternative output drivers (such as HTML or JSON).

If you disagree with my approach, I'm open to discussion and ready to
read your arguments :-)

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.

-- 
Jean Delvare
SUSE L3 Support