[flashrom] [PATCH 1/3] Fix memleaks in dmi.c.

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Sun Sep 23 14:24:14 CEST 2012


On Sun, 23 Sep 2012 14:00:43 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Am 23.09.2012 13:51 schrieb Stefan Tauner:
> > On Sun, 23 Sep 2012 01:30:50 +0200
> > Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
> >
> >>> +static int dmi_shutdown(void *data)
> >>> +{
> >>> +	int i;
> >>> +	for (i = 0; i < ARRAY_SIZE(dmistrings); i++) {
> >>> +		free(dmistrings[i]);  
> >> libflashrom allows calling programmer_init() and programmer_shutdown()
> >> again after one flashrom run. For that case, we want the second run to
> >> have zeroed dmistrings[i]. Please insert
> >> dmistrings[i] = NULL;
> > IIRC i even had this in first. it is not necessary though because they
> > get overwritten by dmi_init() which gets called unconditionally by
> > internal_init().
> 
> Yes, I saw that code too. I am just a bit uncomfortable with leaving
> invalid non-null pointers around.

me too, but i am also uncomfortable with adding redundant code.
i have committed the patch including the nullifying code anyway in
r1604, thanks.

> > i am not convinced, that it is a good idea to null
> > them anyway. note that the array is not initialized at startup either.
> 
> I'm not happy about uninitialized static arrays. That said, I think we
> could kill a few globals if there was a struct with private programmer
> data, containing DMI strings, coreboot names, and other fun stuff. That
> way we could initialize everything in one fell swoop.

my guts tell me this sounds way better than it would actually work
like :)

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner




More information about the flashrom mailing list