Re: [Qemu-devel] [PATCH v2 2/6] block: vmdk - move string allocations from stack to the heap

[Daniel P. Berrange]

On Thu, Jan 22, 2015 at 11:17:35AM +0000, Stefan Hajnoczi wrote:
> On Tue, Jan 20, 2015 at 12:31:29PM -0500, Jeff Cody wrote:
> > @@ -792,12 +792,11 @@ static int vmdk_parse_extents(const char *desc, 
> > BlockDriverState *bs,
> >      const char *p = desc;
> >      int64_t sectors = 0;
> >      int64_t flat_offset;
> > -    char extent_path[PATH_MAX];
> > +    char *extent_path = g_malloc0(PATH_MAX);
> 
> Simpler alternative that has no risk of memory leaks:
> 
>   extent_path = g_malloc0(PATH_MAX);
>   path_combine(extent_path, sizeof(extent_path),
>           desc_file_path, fname);
>   extent_file = NULL;
>   ret = bdrv_open(&extent_file, extent_path, NULL, NULL,
>                   bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
>   g_free(extent_path);

Much better would be to change path_combine so it doesn't need to have
the destination alloc done by the caller. Just have it allocate the
right sized string buffer directly & return that and avoid the madness
of PATH_MAX entirely.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|