Libguestfs - Mar 2016 - Re: [PATCH 3/3] fish: fix btrfs subvolumes display in error case

On Tuesday 01 March 2016 11:17:19 Cédric Bosdonnat
wrote:
> The list of filesystems that is printed when there was an error prints
> the internal mountable string even for the btrfs subvolumes. Let's
> printing a valid -m option value instead.
> ---
>  fish/options.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/fish/options.c b/fish/options.c
> index da5015d..5c09bfb 100644
> --- a/fish/options.c
> +++ b/fish/options.c
> @@ -280,7 +280,20 @@ display_mountpoints_on_failure (const char *mp_device,
>             guestfs_int_program_name);
>  
>    for (i = 0; fses[i] != NULL; i += 2) {
> -    CLEANUP_FREE char *p = guestfs_canonical_device_name (g, fses[i]);
> +    CLEANUP_FREE char *p = NULL;
> +    CLEANUP_FREE char *device = guestfs_mountable_device(g, fses[i]);
> +    CLEANUP_FREE char *volume = guestfs_mountable_subvolume(g, fses[i]);
If guestfs_mountable_subvolume fails, then an error is issued to the
error handler set: you need to push a temporary null error handler to
avoid that:

  CLEANUP_FREE char *subvolume = NULL;
  int subvolume_errno;
  ...
  guestfs_push_error_handler (g, NULL, NULL);
  subvolume = guestfs_mountable_subvolume (g, fses[i]);
  if (subvolume == NULL && guestfs_last_errno (g) != EINVAL) {
    fprintf (stderr,
             _("%s: cannot determine the subvolume for %s: %s
(%d)\n"),
             guestfs_int_program_name, fses[i],
             guestfs_last_error (g), guestfs_last_errno (g));
    exit (EXIT_FAILURE);
  }
  guestfs_pop_error_handler (g);
> +    /* Reformat the internal btrfsvol string into a valid mount option */
> +    if (device && volume && guestfs_last_errno (g) !=
EINVAL) {
With the above, this needs to check device and subvolume only.
> +      if (asprintf (&p, "%s:/:subvol=%s", device, volume) ==
-1) {
> +          perror ("asprintf");
> +          exit (EXIT_FAILURE);
> +      }
> +    } else {
> +      p = guestfs_canonical_device_name (g, fses[i]);
> +    }
Usually there are no curly brackets for blocks of a single instruction.

Thanks,
-- 
Pino Toscano

On Tue, 2016-03-01 at 13:35 +0100, Pino Toscano wrote:
> On Tuesday 01 March 2016 11:17:19 Cédric Bosdonnat wrote:
[...]
> > +      if (asprintf (&p, "%s:/:subvol=%s", device,
volume) == -1) {
> > +          perror ("asprintf");
> > +          exit (EXIT_FAILURE);
> > +      }
> > +    } else {
> > +      p = guestfs_canonical_device_name (g, fses[i]);
> > +    }
> 
> Usually there are no curly brackets for blocks of a single
> instruction.
Even if there is a non-single line block in the same if (or else) part?

--
Cedric

On Tue, Mar 01, 2016 at 08:19:02PM +0100, Cedric Bosdonnat
wrote:
> On Tue, 2016-03-01 at 13:35 +0100, Pino Toscano wrote:
> > On Tuesday 01 March 2016 11:17:19 Cédric Bosdonnat wrote:
> 
> [...]
> 
> > > +      if (asprintf (&p, "%s:/:subvol=%s", device,
volume) == -1) {
> > > +          perror ("asprintf");
> > > +          exit (EXIT_FAILURE);
> > > +      }
> > > +    } else {
> > > +      p = guestfs_canonical_device_name (g, fses[i]);
> > > +    }
> > 
> > Usually there are no curly brackets for blocks of a single
> > instruction.
> 
> Even if there is a non-single line block in the same if (or else) part?
There's not really a rule on this.  As long as the code is clear and
well formatted, then whichever you prefer.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/