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

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

> > > > 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: ...