On Wed, Nov 04, 2015 at 08:26:00AM -0600, Andrew Jones wrote:
> On Wed, Nov 04, 2015 at 11:46:22AM +0100, Lennart Poettering wrote:
> > On Tue, 03.11.15 15:04, Andrew Jones (drjo...@redhat.com) wrote:
> >
> > > The variable 's' is still in scope until we exit the function. We
> > > can't call read_one_line_file on it multiple times without calling
> > > free in between.
> > > ---
> > > src/basic/virt.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/basic/virt.c b/src/basic/virt.c
> > > index 1e10fc755f201..f88bd04bd72ed 100644
> > > --- a/src/basic/virt.c
> > > +++ b/src/basic/virt.c
> > > @@ -176,9 +176,10 @@ static int detect_vm_dmi(void) {
> > >
> > > r = read_one_line_file(dmi_vendors[i], &s);
> > > if (r < 0) {
> > > - if (r == -ENOENT)
> > > + if (r == -ENOENT) {
> > > + free(s);
> > > continue;
> >
> > Note that we use the _cleanup_free_ macro on "s", which uses gcc's
> > "cleanup" attribute to automatically free the variable when it goes
> > out of scope. An explicit free() is hence unnecessary. In fact, it
> > would result in a double-free.
> >
> > We use the "cleanup" logic pretty much everywhere in systemd, in order
> > to keep the error paths manageable.
>
> As I wrote in the commit message 's' is still in scope until we exit
> the function, detect_vm_dmi(). Calling read_one_line_file multiple times
> will reassign 's' with newly allocated buffers each time, losing
> references to the previously allocated buffers. _cleanup_free_ will only
> ensure the last buffer is freed (which is why I didn't put the free() at
> the function exit, but rather above the continue.)
>
> Thanks,
> drew
Crap. Never mind. I just looked again and see that 's' is local to the
loop, not the function... Sorry for the noise.
drew
>
> >
> > Lennart
> >
> > --
> > Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
