[lxc-devel] [PATCH] lxc_monitor: fix memory leak on @fds and close fds

[Arjun Sreedharan]

On 10 June 2015 at 19:57, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> Quoting Serge Hallyn (serge.hallyn at ubuntu.com):
>> Quoting Arjun Sreedharan (arjun024 at gmail.com):
>> > also label and consolidate error conditions for
>> > better readability.
>> >
>> > Signed-off-by: Arjun Sreedharan <arjun024 at gmail.com>
>>
>> My only concern here is that rc in main is a pretty
>> generic name - could you please either put a comment by its
>> definition saying "this is main's return value, do not
>> use it for anything else", or use a different variable
>> name?  (You can add my ack when you resend)

Sure, agreed. I will use @rc_main and resend.

>
> (and of course make close_fds static as per your second posting :)
>
>> Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
>>
>> > ---
>> >  src/lxc/lxc_monitor.c | 50 ++++++++++++++++++++++++++++++++++++--------------
>> >  1 file changed, 36 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
>> > index b3d8912..30b8bfe 100644
>> > --- a/src/lxc/lxc_monitor.c
>> > +++ b/src/lxc/lxc_monitor.c
>> > @@ -71,6 +71,18 @@ Options :\n\
>> >     .lxcpath_additional = -1,
>> >  };
>> >
>> > +void close_fds(struct pollfd *fds, nfds_t nfds)
>> > +{
>> > +   nfds_t i;
>> > +
>> > +   if (nfds < 1)
>> > +           return;
>> > +
>> > +   for (i = 0; i < nfds; ++i) {
>> > +           close(fds[i].fd);
>> > +   }
>> > +}
>> > +
>> >  int main(int argc, char *argv[])
>> >  {
>> >     char *regexp;
>> > @@ -78,7 +90,9 @@ int main(int argc, char *argv[])
>> >     regex_t preg;
>> >     struct pollfd *fds;
>> >     nfds_t nfds;
>> > -   int len, rc, i;
>> > +   int len, rc, rc_snp, i;
>> > +
>> > +   rc = 0;
>> >
>> >     if (lxc_arguments_parse(&my_args, argc, argv))
>> >             return 1;
>> > @@ -119,24 +133,24 @@ int main(int argc, char *argv[])
>> >             ERROR("failed to allocate memory");
>> >             return 1;
>> >     }
>> > -   rc = snprintf(regexp, len, "^%s$", my_args.name);
>> > -   if (rc < 0 || rc >= len) {
>> > +   rc_snp = snprintf(regexp, len, "^%s$", my_args.name);
>> > +   if (rc_snp < 0 || rc_snp >= len) {
>> >             ERROR("Name too long");
>> > -           free(regexp);
>> > -           return 1;
>> > +           rc = 1;
>> > +           goto error;
>> >     }
>> >
>> >     if (regcomp(&preg, regexp, REG_NOSUB|REG_EXTENDED)) {
>> >             ERROR("failed to compile the regex '%s'", my_args.name);
>> > -           free(regexp);
>> > -           return 1;
>> > +           rc = 1;
>> > +           goto error;
>> >     }
>> > -   free(regexp);
>> >
>> >     fds = malloc(my_args.lxcpath_cnt * sizeof(struct pollfd));
>> >     if (!fds) {
>> >             SYSERROR("out of memory");
>> > -           return -1;
>> > +           rc = -1;
>> > +           goto error;
>> >     }
>> >
>> >     nfds = my_args.lxcpath_cnt;
>> > @@ -147,8 +161,9 @@ int main(int argc, char *argv[])
>> >
>> >             fd = lxc_monitor_open(my_args.lxcpath[i]);
>> >             if (fd < 0) {
>> > -                   regfree(&preg);
>> > -                   return 1;
>> > +                   close_fds(fds, i);
>> > +                   rc = 1;
>> > +                   goto cleanup;
>> >             }
>> >             fds[i].fd = fd;
>> >             fds[i].events = POLLIN;
>> > @@ -159,8 +174,8 @@ int main(int argc, char *argv[])
>> >
>> >     for (;;) {
>> >             if (lxc_monitor_read_fdset(fds, nfds, &msg, -1) < 0) {
>> > -                   regfree(&preg);
>> > -                   return 1;
>> > +                   rc = 1;
>> > +                   goto close_and_clean;
>> >             }
>> >
>> >             msg.name[sizeof(msg.name)-1] = '\0';
>> > @@ -182,7 +197,14 @@ int main(int argc, char *argv[])
>> >             }
>> >     }
>> >
>> > +close_and_clean:
>> > +   close_fds(fds, nfds);
>> > +
>> > +cleanup:
>> >     regfree(&preg);
>> > +   free(fds);
>> >
>> > -   return 0;
>> > +error:
>> > +   free(regexp);
>> > +   return rc;
>> >  }
>> > --
>> > 1.7.11.7
>> >
>> _______________________________________________
>> lxc-devel mailing list
>> lxc-devel at lists.linuxcontainers.org
>> http://lists.linuxcontainers.org/listinfo/lxc-devel