[Libvir] Repository for work-in-progress storage patches

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> On Sat, Jan 19, 2008 at 07:09:31PM +0100, Jim Meyering wrote:
...
>>   In storage_backend_loop.c, it looks like vol->target.path can be leaked.
>
> Which function is that in ?  Since originally writing it I've changed all
> error path cleanup code to simply call virStorageVolDefFree(), so there's
> a central function which will free all members, rather than having cleanup
> duplicated all over the place.

This is the state from yesterday:

static int virStorageBackendLoopVolCreate(virConnectPtr conn,
...
    if ((vol->target.path = malloc(5 + strlen(vol->name) + 1)) == NULL) {
        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "target");
        return -1;
    }
    strcpy(vol->target.path, "/dev/");
    strcat(vol->target.path, vol->name);

    if ((vol->key = strdup(vol->target.path)) == NULL) {
        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "storage vol key");
        return -1;
    }

    if (virRun(conn, (char**)cmdargv, NULL) < 0)
        return -1;

    if ((fd = open64(vol->target.path, O_RDONLY)) < 0) {
        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "cannot read path '%s': %d (%s)",
                              vol->target.path, errno, strerror(errno));
        goto cleanup;
    }
...
 cleanup:
    close(fd);
    virStorageBackendLoopVolDelete(conn, pool, vol);
    return -1;
}

-----------------------------
I'd say just do s/return -1/goto cleanup/ after failed strdup
but then you have to be careful to set fd = -1, and avoid calling
"close" on it in that case.