[flashrom] DMI matching patch

Luc Verhaegen libv at skynet.be
Thu Jan 7 12:22:36 CET 2010


On Thu, Jan 07, 2010 at 11:16:14AM +0100, Michael Karcher wrote:
> Am Donnerstag, den 07.01.2010, 10:25 +0100 schrieb Luc Verhaegen:
> > > > I'm not so happy about the static allocation here. Can't we use malloc()
> > > > for the read buffer and strdup() for filling in dmistrings?
> > > Of course we can. On the other hand, I don't really see the point in
> > > using dynamic allocation here. If it's about wasted memory, we should
> > > approach "struct flashchips" first. The fixed-size arrays in it waste a
> > > lot more. Nevertheless, I change it to dynamic allocation. Too bad the
> > > nice getline function that reads one line and allocates dynamically is
> > > GNU-only and not standard C or at least POSIX.
> > This is a run-once tool, the results are not stored, and we do not go 
> > and run dmidecode several times. Everything filling up any memory should 
> > be ran once.
> 
> Sorry, I don't really understand your statement in this context. We do
> not run dmidecode once, but once for each string that might be
> interesting, because the machine-readable "-s" interface of dmidecode
> can only return one string at once. That's six invocations of dmidecode
> per flashrom start. The strings are collected during flashrom
> initialization and kept. The context of what you were replying to was
> whether the 6 dmi strings are stored in a statically allocated buffer or
> in 6 buffers allocated by malloc.

Ok, then each of those 6 strings are only stored once, and are always 
stored (if dmidecode is present), the never need to get used for other 
purposes, they never need to get removed and re-added with other 
contents. They are local to the dmi.c code. I do not see a reason to 
make them dynamically allocated.

> > That's way too contrived for my taste.
>
> That's what I expected, and that's why I didn't commit yet. I won't
> commit until you and Carl-Daniel agree on the necessity of providing
> which string to match, although I slightly prefer the explicit
> specification of the string to match.
 
> > If you really believe it is 
> > necessary, then you should provide an enum entry right in front of the 
> > string.
>
> Yeah. This would make the data structure cleaner...
 
> > But then the board enable table is taking another big hit, and this i 
> > would like to avoid at all cost.
>
> ... but I avoided it for exactly this reason. Especially because it will
> add only one empty column on boards that don't need DMI matching.
> 
> > We already have pci ids and pci subsystem ids, this is just an extra
> > bit of info that will be the last in precedence for any match.
>
> Looking at the Asus P5A example (I know that this board is severly
> outdated, which might make this point less strong) there are no
> subsystem IDs at all in it. So when we start looking for DMI info we
> don't have any indication yet that we are on an Asus board. If we really
> have good subsystem IDs, we don't need DMI.

Yeah, maybe a few of the other boards which can only be matched 
with -m can also be improved this way.

Luc Verhaegen.




More information about the flashrom mailing list