[Bug 495564] Review Request: libguestfs - Access and modify virtual machine disk images

bugzilla at redhat.com bugzilla at redhat.com
Wed May 6 12:13:40 UTC 2009


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=495564





--- Comment #13 from Jim Meyering <meyering at redhat.com>  2009-05-06 08:13:39 EDT ---
This realloc (from guestfsd.c) leaks upon failure:

      if (r > 0 && stdoutput) {
 so_size += r;
 *stdoutput = realloc (*stdoutput, so_size);
 if (*stdoutput == NULL) {
   perror ("realloc");
   *stdoutput = NULL;
   continue;
 }
 memcpy (*stdoutput + so_size - r, buf, r);
      }

Also, this stmt is unnecessary:

   *stdoutput = NULL;

Same thing with unnecessary code below:

    if (*stdoutput == NULL) {
      perror ("realloc");
      *stdoutput = NULL;
    } else

and with stderror:

    if (*stderror == NULL) {
      perror ("realloc");
      *stderror = NULL;

More importantly, it looks like this function can return
0 even when it has set *stdoutput to NULL, and that would
make the following from file.c deference out==NULL:

    r = command (&out, &err, "file", "-bsL", buf, NULL);
    if (freeit) free (buf);

    if (r == -1) {
      free (out);
      reply_with_error ("file: %s: %s", path, err);
      free (err);
      return NULL;
    }
    free (err);

    /* We need to remove the trailing \n from output of file(1). */
    len = strlen (out);
    if (out[len-1] == '\n')
      out[len-1] = '\0';

Also, so_size and se_size become invalid upon failed realloc.
You should change it so that they are increased only if realloc
succeeds.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.




More information about the package-review mailing list