Re: [Xen-devel] [PATCH] libxl: Cleanup, use LOG*, GCSPRINTF and CTX macros in libxl_device.c

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-11-05 at 11:48 +0000, George Dunlap wrote:
> On Mon, Nov 4, 2013 at 9:43 PM, Alexandra Sava
> <alexandrasava18@xxxxxxxxx> wrote:
> > Signed-off-by: Alexandra Sava <alexandrasava18@xxxxxxxxx>
> 
> Thanks for submitting this, Alexandra -- it's the kind of thing that
> is important but we rarely make the time to do.
> 
> I don't think the description is quite accurate: you're not cleaning
> up existing use of those macros, but you're updating code to use the
> new macros.

FWIW I read this as "Cleanup: use LOG*" which I think is accurate.

> > @@ -86,7 +86,7 @@ out:
> >  int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
> >          libxl__device *device, char **bents, char **fents, char **ro_fents)
> >  {
> > -    libxl_ctx *ctx = libxl__gc_owner(gc);
> > +    libxl_ctx *ctx = CTX;

The intended use of CTX is inline in the code, not to create a lower
case convenience. This is probably a separate cleanup though.

> >      char *frontend_path, *backend_path;
> >      struct xs_permissions frontend_perms[2];
> >      struct xs_permissions ro_frontend_perms[2];

>  @@ -192,24 +191,21 @@ static int disk_try_backend(disk_try_backend_args *a,
> >          if (libxl__try_phy_backend(a->stab.st_mode))
> >              return backend;
> >
> > -        LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend phy"
> > -                   " unsuitable as phys path not a block device",
> > -                   a->disk->vdev);
> > +        LOG(DEBUG, "Disk vdev=%s, backend phy unsuitable as phys path not 
> > a block device",
> 
> This line is now more than 80 characters long -- please leave it
> broken down so as to be less than 80 characters long.

Or maybe re-break it at the punctuation so it is greppable (my main
bugbear with the 80 column rule is that it causes people to break these
strings in a way which makes them unfindable...)

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel