On Fri, Aug 05, 2022 at 11:57:04AM +0200, Jean Delvare wrote: > 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.
My concerns are at a lower level. There appears to be a difference in interpretation between Linux and Free BSD tool chain on whether passing a NULL as format is legal. I did a quick read of the fprintf function in the Ansi C 99 standard and I'm of the believe that NULL is not legal. But even if my interpretation is incorrect, there is still a difference between the two and I am not going to be testing under Free BSD. So, I'd probably go ahead an proactively change functions like pr_handle_name, pr_attr, etc., to check format like pr_list_start does to protect against this type of issue getting hit in the future. > > 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 :-) I'm fine w/ your thinking here. My concerns are over a different aspect of the issue we hit. > > 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 -- ----------------------------------------------------------------------------- Jerry Hoemann Software Engineer Hewlett Packard Enterprise ----------------------------------------------------------------------------- _______________________________________________ https://lists.nongnu.org/mailman/listinfo/dmidecode-devel
