Re: [Xen-devel] [PATCH] minios: Fix xenbus_rm() calls in frontend drivers

[Matt Wilson <msw@xxxxxxxxx>]

On Fri, Sep 06, 2013 at 10:00:29AM +0100, Ian Campbell wrote:
> On Thu, 2013-09-05 at 11:06 -0700, Matt Wilson wrote:
> > On Thu, Sep 05, 2013 at 10:17:21AM +0100, Ian Campbell wrote:
> > > On Wed, 2013-09-04 at 17:25 -0700, Matt Wilson wrote:
[...]
> > > >      char path[strlen(dev->backend) + 1 + 5 + 1];
> > > > -    char nodename[strlen(dev->nodename) + 1 + 5 + 1];
> > > > +    char nodename[strlen(dev->nodename) + 1 + 13 + 1];
> > > 
> > > These changes don't seem to be covered by the commit message? I assume
> > > they relate to the length of the longest suffix which we are appending,
> > > perhaps using strlen("some-string-const") would make this more obvious?
> > 
> > Yes, those are length related changes. I'd like to keep the code as-is
> > (following the established pattern) for this round 
> 
> Why? What is the benefit to keeping it this way when you are changing it
> anyway?

This should be cleaned up everywhere in a separate patch. There are
many other places where mini-os uses the existing pattern.

[msw@carbon mini-os]$ git grep ') + 1 +' | wc -l
27

http://wiki.xenproject.org/wiki/Submitting_Xen_Patches#Break_down_your_patches

"Don't mix clean-up patches (which make things look prettier or move
things round but don't change functionality) with code-change
patches. Clean-up patches should be clearly marked as having no
functional changes."

--msw

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