libsmbios (ready to deliver?)
Michael_E_Brown at Dell.com
Tue Jul 26 16:59:38 CDT 2005
New comments in Red... :-)
Extra stuff cut out.
Gratuitous: Outlook really, really sucks for collaborative email
reply/response back-and-forth conversations. Everything gets too
colorful and top-posting encourages bad habits.
> -----Original Message-----
> From: Fettig, Tim - Authorized Dell Representative
> From: Brown, Michael E
> It appears that you "hand generate" your XML output. I want to
> understand why you do this instead of using the existing xerces
> library for XML generation.
> The initial requirement for my coding effort was to accomplish XML
> tagged output WITHOUT the Xerces library. DSET has a lot of
> restrictions to package size with the Xerces DLL adding a 2.x MB
> penalty. Most of the XML generated within DSET's utilities is done
> sans any DOM objects. Another reason for "hand generation" was to use
> the existing table element definitions for the tags. These strings
> typically were human oriented labels containing embedded spaces which
> are not allowed as tags in XML.
> Once it was established that the method employed for libsmbios to
> assign textual labels/decoding of table values was via Xerces and your
> pre-compiled header file, there was no effort made to reorient the
> XML tagging to leverage a DOM handler. This could be modified going
> forward since libsmbios is so tightly coupled to DOM processing and
> there is no expectation that this would change.
It may not be immediately obvious from the Windows build, but the
"precompiled header" is simply the file "doc/smbios23.xml" run through a
simple python script (build/scripts/makeXmlHeader.py) to generate the
file. In the Linux build, this file is generated every build. I cannot
guarantee (and don't want to have to enforce) that every Windows box
will have Python, so I just include the file in the tarball.
If you want to update the XML file, it can be updated at runtime by
supplying an updated doc filename to the smbiosxml factory. There is an
xsd file in doc/external-specs/smbios.xsd, but I'm not sure how well the
smbios23.xml file adheres to the xsd. I "borrowed" these two files from
a prior SMBIOS parsing library internal to Dell that was never released
(called 'smbq' and IIRC written by Craig Lowry). This is basically the
only thing that survived, as I took no code from smbq, just these XML
If you wish to add anything to these files, go ahead. They are not
completely up-to-date for the latest 2.4 SMBIOS spec.
> Last, and this is the largest issue I have with the code, the
> main addition to the code is streamify2() in SmbiosXml.cpp, which is
> one single function consisting of over 440 lines of code. There are a
> very small handful of existing functions in libsmbios that are around
> 60 lines. This is about the maximum function size that I allow, unless
> there are extremely compelling reasons to the contrary. Even then, at
> 60 lines, the function must be very well commented. A reasonable
> maximum size for a function is 20 to 30 lines of code. Since this your
> biggest issue, I want to understand this clearly:
> Keeping functions to 60 lines or less would mean to me, breaking
> out each element type (i.e. BITSTREAM would have one for
> WORD,DWORD,BYTE, etc) to be processed by it's own function?
Yes, pretty much.
> . From my review of the XML structure used to decode the table data,
> it seemed that each type was not only unique in it's size but also in
> what I could determine were it's rules of use (I did not find
> documentation that spoke to these rules). This made for a very
> unnecessarily complicated chunk of code since the data being populated
> in the table did not always require or have a matching entry in the
> XML table. I assumed that this was because the XML used in the header
> was still a "work in progress" and that once it had a good XSD or DTD
> assigned to it that this code could be very easily made to resolve
> many types via a single function (in specific I expected to see a
> place holder for bits not used (ala BYTE type) instead of using "lsb"
> and "msb" attributes to indicate that bit positions contained more
> than a single bit's worth of data) . (Do you have an XSD that
> validates your XML structure header file? This would be very helpful
> in getting this code size down.)
XSD mentioned above. Keep in mind, though, that I only used the XML file
that I have because somebody else already had done the hard work and I
didn't want to re-invent the wheel. :-)
In many ways, the existing XML definition file is not adequate, but I
have not had any real users hit these inadequacies yet. You are the
first people to really use the XML stuff heavily. If the XML doesn't
fit, it can easily be changed, as long as you submit patch to fix the
existing doc/smbios23.xml file to fit the new definition.
In particular, your critique of the BITFIELD definition is very well
placed. I have the exact same critique. But, since I have no actual
users of this particular piece of uglyness, there has been no pressing
need to "fix" it. We just coded up (Actually, I bribed David Schmidt
into coding) what would seem to cover mostly what would be needed should
anybody have a pressing need for it. If you wish to propose a different
XML definition for the bitfield types, I would be very happy to hear it.
> Also in this area, if Matt does not object to relying on the
> Xerces DOM to handle XML creation (although I did not see any method
> of creating a "good" XML tag from a string containing spaces and other
> invalid tag characters), this whole section of code could be collapsed
> considerably. So re-coding this to use Xerces would not only allow
> smaller functions but, (with the addition of your XSD) allow us to
> create the entire XML document and validate it before writing a single
> character out. Currently we toss it out as we make it and validate it
> outside of libsmbios.
If you are going through the trouble of actually generating something
that looks like XML and quacks like XML, why not just go all the way and
follow the rest of those pesky XML rules as well?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the libsmbios-devel