dmidecode-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: dmidecode --json


From: Jean Delvare
Subject: Re: dmidecode --json
Date: Mon, 25 Nov 2024 17:50:59 +0100

Hi Jiri,

On Tue, 4 Jul 2023 11:11:13 +0200, Jiri Hnidek wrote:
> On Mon, Jul 3, 2023 at 6:57 PM Erwan Velu <erwanaliasr1@gmail.com> wrote:
> > 4/ be careful of the code indent in your PR, you're breaking the coding
> > style
> 
> Oh no. I thought that my IDE is smart enough and it is able to detect the
> style of indentation of imported projects and alter settings for
> related projects. Never mind. I will fix it. Will it be enough to fix
> it in next commit or is it necessary to fix it in all commits?

I know it is more work but I insist on having the cleanest possible
history, so issues found during review, including coding style issues,
should be amended in the original commit.

> Open questions:
> 1) What about "handle" values? Could it be a number? Or should I convert it
> to string with hexadecimal representation? I lean towards hex values to be
> consistent with text format and compatible with jc.

Apparently original JSON does not support hexadecimal values as a
number, so indeed you would need to use strings if you want to use
hexadecimal values. While using (decimal) numbers would seems more
natural for JSON, sticking to hexadecimal (and thus strings) would be
closer to the text output of dmidecode. As the dmidecode output already
includes many hexadecimal values which we definitely don't want to
convert to decimal (bitfields, addresses), doing the same for handles
should make things more simple and more consistent, so I recommend that
you do it that way.

> 2) There is nothing printed in pr_info() and pr_comment(), when --json is
> used. Is it acceptable?

This is not a blocker, but we may think about the best thing to do with
these strings. I seem to understand that the JSON format has no
provision for comments, but maybe they can be included as data in a
separate structure. Some of the comment strings are not very valuable,
but things like the SMBIOS version and the version of dmidecode can be
useful to interpret the data afterwards.

Note that pr_comment() and pr_info() were originally tailored for the
text output format, their exact semantics aren't very clear nor carved
in stone. So if finer-grained functions are needed to better integrate
with the JSON output, this is an option.

> 3) I converted all printf() functions from dmidecode.c to pr_printf() and
> it does print anything, when --json is used now. I would like to put
> such information to output somehow, when --json is used, but I have
> to admit that this will not be easy. Any suggestions?

I must look into the code to figure out what you did exactly and why
you had to do it. In theory, all open-coded printfs had already been
removed from the regular dmidecode output (the remaining ones were for
special modes like --string, which are mutually exclusive with JSON
output), so alternative output formats were supposed to just hook into
pr_*() functions. If that wasn't the case then this is a bug which
should be fixed beforehand.

-- 
Jean Delvare
SUSE L3 Support



reply via email to

[Prev in Thread] Current Thread [Next in Thread]