On Mon, Aug 06, 2018 at 03:43:34PM +0200, Jean Delvare wrote: > On Sun, 2018-08-05 at 12:49 -0400, Neil Horman wrote: > > On Sat, Aug 04, 2018 at 10:59:39PM +0200, Jean Delvare wrote: > > > But now that you are raising the point again, I took a look at the most > > > recent specification and found the following note (apparently added in > > > 2.8.0, but I did not pay attention at that time): > > > > > > NOTE The Entry Point Structure and all SMBIOS structures assume a > > > little-endian ordering convention, unless > > > explicitly specified otherwise, i.e., multi-byte numbers (WORD, DWORD, > > > etc.) are stored with the low-order > > > byte at the lowest address and the high-order byte at the highest address. > > > > Ok, cool, so this suggests that we should be using the ntoh* macros > > consistently. > > I'm not sure how ntoh* would help here. We now know that the DMI table > is using little-endian. Network byte order, on the other hand, is big- > endian. Did you mean le*toh functions? > Yes, sorry, I'm specifically referring to the endian conversion functions in endian.h (leXtoh and htoleX). They are not strictly speaking posix compliant, but are fairly universally available.
> 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. > > > Which means I got it wrong and types.h needs to be revisited. This also > > > suggests that nobody ever tried dmidecode on a big-endian system, or > > > they would have noticed. > > > > The only system I know of that would have been consistently big endian I > > think > > would have been old pre power-9 systems, which IIRC never supported smbios. > > Correct. > > > > > That said, dmidecode can also read from a dump > > > > file, so its possible that someone might take a dump on an intel > > > > system, and go > > > > read it on an old power system. > > > > > > This is a good point. The endianness of the original system is not > > > recorded in the dump, so if the data endianness would depend on the > > > original host, we would be in trouble. But as it seems now clear that > > > all DMI data must be little-endian, we should be on the safe side. It's > > > currently broken, but easy to fix. > > > > Agreed, if we just assume consistent little endian from the source (be it > > in-memory or file), we can move forward. > > I think that's where we are header, yes. I made some preliminary tests > on a POWER7 system. I have a patch almost ready, I'll post it for > review soon. > > > > Reading a dump from a powerpc system might be challenge because > > > typically distributions don't package dmidecode on architectures which > > > do not implement SMBIOS. But I could try building it from sources on > > > the machine directly. > > > > > > > I would say, it wouldn't hurt to update dmidecode to be endian aware, > > > > but such > > > > an update is likely outside the scope of this patch. > > > > > > Actually we need to update it to be endian agnostic ;-) but yes, that > > > would be done in a separate patch. > > > > Ok, so are you comfortable with this patch as it is? I'm happy to start a > > review of the code to consistently use the endian conversion macros if you > > like. > > 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. Neil > Thanks, > -- > Jean Delvare > SUSE L3 Support > _______________________________________________ https://lists.nongnu.org/mailman/listinfo/dmidecode-devel
