On Tue, Jun 21, 2011 at 9:33 AM, Jaroslav Skarvada <jskarvad(a)redhat.com> wrote:
----- Original Message -----
> On Tue, Jun 21, 2011 at 8:17 AM, Russell Bryant
> <russell(a)russellbryant.net> wrote:
> > On Sat, Jun 4, 2011 at 7:54 AM, Jaroslav Škarvada
> > <jskarvad(a)redhat.com> wrote:
> >
> >> - _instance.setProperty("hostname", mh_hostname());
> >> - _instance.setProperty("uuid", mh_uuid());
> >> + _instance.setProperty("hostname", str = (char *)mh_hostname());
> >> + free(str);
> >> + _instance.setProperty("uuid", str = (char *)mh_uuid());
> >> + free(str);
> >
> > These changes will cause a crash. These functions don't leak memory.
> > They do return dynamically allocated memory, but the memory is only
> > allocated a single time and then is saved off to a static pointer.
> > Future invokations of the function return the saved off buffer.
>
Right, trust me or not but I do not mention the static :)
Sorry for noise.
It's not noise! No problem at all ... that's what peer review is for. :-)
> To clarify, what I quoted was just an example. I believe this
applies
> to all of the changes.
What about the mh_network_get_ip_address, mh_network_get_mac_address
and some explicit list frees?
I'll have to go back and look at the patch tonight. I guess I didn't
look close enough on the first pass. Those do sound like legitimate
leaks, though. It also sounds like there might be some overlap
between those changes and some memory leak fix patches I have posted
over the last few days. I modified those two functions and fixed a
bunch of list leaks. Take a look and see if there is anything I
missed and if we should combine our changes.
--
Russell Bryant