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 > > Lennart > > -- > Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel