Re: [dmidecode] [PATCH] update dmidecode to parse Modern Management Controller blocks

[Jean Delvare]

Hi Neil,

On Mon, 6 Aug 2018 16:07:56 -0400, Neil Horman wrote:
> On Mon, Aug 06, 2018 at 03:43:34PM +0200, Jean Delvare wrote:
> > As I understand it, that's pretty much what I open-coded with my macros
> > in types.h? I'm not sure what would be the benefit of switching to
> > le*toh functions instead. Using WORD() and DWORD() has the advantage
> > that these are the terms used in the SMBIOS specification, so it makes
> > it easier to match the code with the specification, in my opinion. Do
> > you think differently?
>
> I think you missunderstood what I was suggesting.
> 
> the WORD and DWORD macros are exacty what map to leXtoh in types.h.
> What I'm saying is, instead of maintaining a reinvented wheel, why not convert
> types.h such that they only contain these defintions:
> 
> #define WORD(x) le16toh(x)
> #define DWORD(x) le32toh(x)
> #deine QWORD(x) le64toh(x)
>  
> That seems far more concise than maintaining a reimplementation of those 
> macros,
> and still maintains correlation to the spec.

That's tempting, however it does not seem like these macros are dealing
with unaligned access. If we are running on a little-endian machine
which does not support unaligned memory access (hello IA-64!), we end
up with the following definitions:

#  define le16toh(x) (x)
#  define le32toh(x) (x)
#  define le64toh(x) (x)

which will surely crash. So we still need our own macros for the
ALIGNMENT_WORKAROUND case. And then I don't think there is much benefit
in using le*toh macros in one case, and custom code in the other, as
this is the same amount of code we have now?

Also, endian.h doesn't seem to be available on Solaris? I no longer
have access to such system for proper testing, but at least online man
pages don't have any reference to it.

> > (...)
> > I did not read your patch yet, will do when I finally have time. You
> > should be using WORD() etc like the rest of the code is doing, for
> > consistency.
>
> I'll send a v3 then, the system I was validating this on just died on me.  As
> soon as I get it back up, I'll test a version with the WORD/DWORD macros used.

May I suggest that you dump the SMBIOS table of said system to a file,
so you can test your code on another host using --from-dump, thus no
longer depending on the availability of said system? It would be great
if you are also able to share that dump with me, so that I can add it
to my test suite.

Thanks,
-- 
Jean Delvare
SUSE L3 Support