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.