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

[Matt Wilson <msw@xxxxxxxxx>]

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:
> > From: Ben Cressey <bcressey@xxxxxxxxxx>
> > 
> > The commit "minios: refactor xenbus state machine" caused "/state" to
> > be appended to the local value of nodename. Previously the nodename
> > variable pointed to dev->nodename.
> > 
> > The xenbus_rm() calls were not updated to reflect this change, and
> > refer to paths that do not exist.
> > 
> > For example, shutdown_blkfront() for vbd 2049 would issue these calls:
> >     xenbus_rm(XBT_NIL, "device/vbd/2049/state/ring-ref");
> >     xenbus_rm(XBT_NIL, "device/vbd/2049/state/event-channel");
> > 
> > This patch restores the previous behavior, issuing these calls
> > instead:
> >     xenbus_rm(XBT_NIL, "device/vbd/2049/ring-ref");
> >     xenbus_rm(XBT_NIL, "device/vbd/2049/event-channel");
> > 
> > In addition, remove cases where the error message pointer is already
> > NULL and is then set to NULL. These are harmless, but suggest
> > incorrect practice: the pointer should be passed to free() to
> > deallocate memory prior to reassignment.
> 
> Seems sensible to me.
> 
> > Signed-off-by: Ben Cressey <bcressey@xxxxxxxxxx>
> > Reviewed-by: Matt Wilson <msw@xxxxxxxxxx>
> > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > Cc: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
> > Signed-off-by: Matt Wilson <msw@xxxxxxxxxx>
>  
> >      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 unless Stefano
objects. I wouldn't object to a commit message adjustment from the
committer.

--msw

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