Re: [libvirt] PATCH: 15/28: Reduce return points for storage driver

[Jim Meyering <jim meyering net>]

"Daniel P. Berrange" <berrange redhat com> wrote:
> This patch reduces the number of return points in the storage driver
> methods
...
> diff --git a/src/storage_driver.c b/src/storage_driver.c
...
> @@ -893,7 +924,7 @@ storagePoolListVolumes(virStoragePoolPtr
>
>   cleanup:
>      for (n = 0 ; n < maxnames ; n++)
> -        VIR_FREE(names[i]);
> +        VIR_FREE(names[n]);
>
>      memset(names, 0, maxnames);
>      return -1;

This might be worth putting in a separate bug-fix patch.
At first I thought this was fixing a serious bug,
but you can see that i is always smaller than maxnames,
so the fix is just plugging a leak.

However, in looking at this I spotted a real problem:
There are numerous statements like this:

    memset(names, 0, maxnames);

That zeros out only 1/4 or 1/8 of the memory it should.
It should be doing this:

    memset(names, 0, maxnames * sizeof (*names));

These bugs are independent of your 28-part patch, Dan,
i.e., also on the trunk:

  $ git grep memset.names|grep -v sizeof
  src/storage_driver.c:    memset(names, 0, nnames);
  src/storage_driver.c:    memset(names, 0, nnames);
  src/storage_driver.c:    memset(names, 0, maxnames);
  src/storage_driver.c:    memset(names, 0, maxnames);
  src/test.c:    memset(names, 0, maxnames);
  src/test.c:    memset(names, 0, maxnames);

I'll post the fix (relative to the trunk) separately.