[Libguestfs] [PATCH v4 2/3] do_btrfs_subvolume_list: fix a bad return value

[Chen, Hanxiao]

Hi,

> -----Original Message-----
> From: libguestfs-bounces at redhat.com [mailto:libguestfs-bounces at redhat.com] On
> Behalf Of Pino Toscano
> Sent: Thursday, June 18, 2015 4:32 PM
> To: libguestfs at redhat.com
> Subject: Re: [Libguestfs] [PATCH v4 2/3] do_btrfs_subvolume_list: fix a bad return
> value
> 

[snip]

> > >
> >
> > If we succeeded at malloc(3) but failed at calloc(3),
> > we will goto error.
> >
> > At this time we've got a space with uninitialized data because of malloc(3),
> > but no space for guestfs_int_btrfsqgroup_list_val.
> > When processing  in label error, we could not know:
> > ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path
> > is a valid address.
> >
> > 1) One solution is use calloc to replace the first malloc.
> > Then:
> >   if (ret-> guestfs_int_btrfssubvolume_list_val)
> >     for (...)
> >
> > It costs more codes.
> >
> > 2) use the current solution
> >
> > I think the process in this patch should be a choice.
> > How do you think?
> 
> If calloc (nr_subvolumes, sizeof (struct guestfs_int_btrfssubvolume))
> fails, then ret->guestfs_int_btrfssubvolume_list_val is already a null
> pointer, which means you can just check for it as you do in (1) above,
> with no need to switch from malloc to calloc.
> 
> The other alternative is to use more labels for error conditions in a
> symmetric way, like:
> 
>  ptr1 = malloc (...);
>  if (ptr1 == NULL)
>    goto error1;
> 
>  ptr1->subptr1 = malloc (...);
>  if (ptr1->subptr1 == NULL)
>    goto error2;
> 
>  ptr1->subptr2 = malloc (...);
>  if (ptr1->subptr2 == NULL)
>    goto error3;
> 
>  ...
> 
>  error3:
>   free (ptr1->subptr1);
>  error2:
>   free (ptr1);
>  error1:
>   ...
> 

Thanks for your detail clarification and kindly patience.

Although my patch could work, I'll send a new version as your comments.

Regards,
- Chen