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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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?



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