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. _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
