Hi Rich, I ran coverity against febootstrap's head and it spotted four leaks. This fixes them. The first patch plugs three. The second attempts to make the add_link function do what I'm guessing it was intended to do. As is, it was a no-op.>From 7c2ff55613598a1295e213cef36600ad61da7ed6 Mon Sep 17 00:00:00 2001From: Jim Meyering <meyering at redhat.com> Date: Tue, 10 Jan 2012 16:55:49 +0100 Subject: [PATCH 1/2] helper: plug three small leaks * helper/ext2initrd.c (read_module_deps): Don't leak filename. (ext2_make_initrd): Don't leak "outfile". * helper/utils.c (load_file): Don't leak a file pointer. --- helper/ext2initrd.c | 4 +++- helper/utils.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/helper/ext2initrd.c b/helper/ext2initrd.c index dedc1e8..8b64e0f 100644 --- a/helper/ext2initrd.c +++ b/helper/ext2initrd.c @@ -1,5 +1,5 @@ /* febootstrap-supermin-helper reimplementation in C. - * Copyright (C) 2009-2011 Red Hat Inc. + * Copyright (C) 2009-2012 Red Hat Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -115,6 +115,7 @@ ext2_make_initrd (const char *modpath, const char *initrd) FILE *f = fopen (outfile, "w"); if (f == NULL) error (EXIT_FAILURE, errno, "failed to create modules list (%s)", outfile); + free (outfile); FILE *pp = popen (cmd, "w"); if (pp == NULL) error (EXIT_FAILURE, errno, "failed to create pipe (%s)", cmd); @@ -175,6 +176,7 @@ read_module_deps (const char *modpath) char *filename = xasprintf ("%s/modules.dep", modpath); FILE *fp = fopen (filename, "r"); + free (filename); if (fp == NULL) error (EXIT_FAILURE, errno, "open: %s/modules.dep", modpath); diff --git a/helper/utils.c b/helper/utils.c index 81b300a..3308c3c 100644 --- a/helper/utils.c +++ b/helper/utils.c @@ -1,5 +1,5 @@ /* febootstrap-supermin-helper reimplementation in C. - * Copyright (C) 2009-2010 Red Hat Inc. + * Copyright (C) 2009-2010, 2012 Red Hat Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -306,6 +306,7 @@ load_file (const char *filename) line[len-1] = '\0'; add_string (&lines, &n_used, &n_alloc, line); } + fclose (fp); add_string (&lines, &n_used, &n_alloc, NULL); return lines; -- 1.7.9.rc0.13.gbee72>From f03d46334bb318fad7a7b71900d6a3c11586b68f Mon Sep 17 00:00:00 2001From: Jim Meyering <meyering at redhat.com> Date: Tue, 10 Jan 2012 17:04:52 +0100 Subject: [PATCH 2/2] helper: fix no-op add_link function * helper/ext2cpio.c (add_link): Don't leak just-allocated buffer. Instead, link it into links_head list. --- helper/ext2cpio.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/helper/ext2cpio.c b/helper/ext2cpio.c index 2e8258f..82cc3b4 100644 --- a/helper/ext2cpio.c +++ b/helper/ext2cpio.c @@ -1,5 +1,5 @@ /* febootstrap-supermin-helper reimplementation in C. - * Copyright (C) 2009-2010 Red Hat Inc. + * Copyright (C) 2009-2010, 2012 Red Hat Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -348,6 +348,8 @@ add_link (ext2_ino_t real_ino) p->minor = dev_minor; p->major = dev_major; p->real_ino = real_ino; + p->next = links_head; + links_head = p; } static void -- 1.7.9.rc0.13.gbee72
On Tue, Jan 10, 2012 at 05:21:17PM +0100, Jim Meyering wrote:> Hi Rich, > > I ran coverity against febootstrap's head and it spotted four leaks. > This fixes them. The first patch plugs three. > The second attempts to make the add_link function do what I'm > guessing it was intended to do. As is, it was a no-op. > > >From 7c2ff55613598a1295e213cef36600ad61da7ed6 Mon Sep 17 00:00:00 2001 > From: Jim Meyering <meyering at redhat.com> > Date: Tue, 10 Jan 2012 16:55:49 +0100 > Subject: [PATCH 1/2] helper: plug three small leaks > > * helper/ext2initrd.c (read_module_deps): Don't leak filename. > (ext2_make_initrd): Don't leak "outfile". > * helper/utils.c (load_file): Don't leak a file pointer. > --- > helper/ext2initrd.c | 4 +++- > helper/utils.c | 3 ++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/helper/ext2initrd.c b/helper/ext2initrd.c > index dedc1e8..8b64e0f 100644 > --- a/helper/ext2initrd.c > +++ b/helper/ext2initrd.c > @@ -1,5 +1,5 @@ > /* febootstrap-supermin-helper reimplementation in C. > - * Copyright (C) 2009-2011 Red Hat Inc. > + * Copyright (C) 2009-2012 Red Hat Inc. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -115,6 +115,7 @@ ext2_make_initrd (const char *modpath, const char *initrd) > FILE *f = fopen (outfile, "w"); > if (f == NULL) > error (EXIT_FAILURE, errno, "failed to create modules list (%s)", outfile); > + free (outfile); > FILE *pp = popen (cmd, "w"); > if (pp == NULL) > error (EXIT_FAILURE, errno, "failed to create pipe (%s)", cmd); > @@ -175,6 +176,7 @@ read_module_deps (const char *modpath) > > char *filename = xasprintf ("%s/modules.dep", modpath); > FILE *fp = fopen (filename, "r"); > + free (filename); > if (fp == NULL) > error (EXIT_FAILURE, errno, "open: %s/modules.dep", modpath); > > diff --git a/helper/utils.c b/helper/utils.c > index 81b300a..3308c3c 100644 > --- a/helper/utils.c > +++ b/helper/utils.c > @@ -1,5 +1,5 @@ > /* febootstrap-supermin-helper reimplementation in C. > - * Copyright (C) 2009-2010 Red Hat Inc. > + * Copyright (C) 2009-2010, 2012 Red Hat Inc. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -306,6 +306,7 @@ load_file (const char *filename) > line[len-1] = '\0'; > add_string (&lines, &n_used, &n_alloc, line); > } > + fclose (fp); > > add_string (&lines, &n_used, &n_alloc, NULL); > return lines; > -- > 1.7.9.rc0.13.gbee72ACK, although leaks in febootstrap-supermin-helper are deliberate (see febootstrap.git/helper/README for the rationale). Jim do you have a github account? Please send me the username so I can add you to the github "collaborators" list.> >From f03d46334bb318fad7a7b71900d6a3c11586b68f Mon Sep 17 00:00:00 2001 > From: Jim Meyering <meyering at redhat.com> > Date: Tue, 10 Jan 2012 17:04:52 +0100 > Subject: [PATCH 2/2] helper: fix no-op add_link function > > * helper/ext2cpio.c (add_link): Don't leak just-allocated buffer. > Instead, link it into links_head list. > --- > helper/ext2cpio.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > diff --git a/helper/ext2cpio.c b/helper/ext2cpio.c > index 2e8258f..82cc3b4 100644 > --- a/helper/ext2cpio.c > +++ b/helper/ext2cpio.c > @@ -1,5 +1,5 @@ > /* febootstrap-supermin-helper reimplementation in C. > - * Copyright (C) 2009-2010 Red Hat Inc. > + * Copyright (C) 2009-2010, 2012 Red Hat Inc. > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -348,6 +348,8 @@ add_link (ext2_ino_t real_ino) > p->minor = dev_minor; > p->major = dev_major; > p->real_ino = real_ino; > + p->next = links_head; > + links_head = p; > }Scary bug ... ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Jim Meyering wrote:> I ran coverity against febootstrap's head and it spotted four leaks. > This fixes them. The first patch plugs three. > The second attempts to make the add_link function do what I'm > guessing it was intended to do. As is, it was a no-op....> Subject: [PATCH 1/2] helper: plug three small leaks > > * helper/ext2initrd.c (read_module_deps): Don't leak filename. > (ext2_make_initrd): Don't leak "outfile". > * helper/utils.c (load_file): Don't leak a file pointer....> Subject: [PATCH 2/2] helper: fix no-op add_link function > > * helper/ext2cpio.c (add_link): Don't leak just-allocated buffer. > Instead, link it into links_head list.FYI, after a go-ahead from Rich, I've pushed these two.
Possibly Parallel Threads
- [PATCH 1/3] febootstrap/helper/init: make sure /proc is mounted into chroot.
- builder-ubuntu febootstrap success af9f9305a0a48829392a57d24aee30978b449d1d
- [PATCH supermin] supermin: update out-dated comments
- builder-debian febootstrap success e56ae34bcfc3e355dc591b4bd99bbe8e593d33af
- [PATCH supermin v4] Supermin 5 rewrite.