From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Wilson Subject: [PATCH] minios: Fix xenbus_rm() calls in frontend drivers Date: Wed, 4 Sep 2013 17:25:20 -0700 Message-ID: <1378340720-8653-1-git-send-email-msw@linux.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VHNOG-0006pg-DR for xen-devel@lists.xenproject.org; Thu, 05 Sep 2013 00:25:40 +0000 Received: by mail-pd0-f169.google.com with SMTP id r10so1047093pdi.28 for ; Wed, 04 Sep 2013 17:25:36 -0700 (PDT) List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xenproject.org Cc: Ben Cressey , Samuel Thibault , Matt Wilson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org From: Ben Cressey 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. Signed-off-by: Ben Cressey Reviewed-by: Matt Wilson Cc: Stefano Stabellini Cc: Samuel Thibault Signed-off-by: Matt Wilson --- extras/mini-os/blkfront.c | 11 +++++------ extras/mini-os/fbfront.c | 41 +++++++++++++++++++---------------------- extras/mini-os/netfront.c | 19 +++++++++---------- extras/mini-os/pcifront.c | 11 +++++------ 4 files changed, 38 insertions(+), 44 deletions(-) diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c index f4283a9..ab58102 100644 --- a/extras/mini-os/blkfront.c +++ b/extras/mini-os/blkfront.c @@ -254,7 +254,7 @@ void shutdown_blkfront(struct blkfront_dev *dev) XenbusState state; char path[strlen(dev->backend) + 1 + 5 + 1]; - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; + char nodename[strlen(dev->nodename) + 1 + 13 + 1]; blkfront_sync(dev); @@ -289,7 +289,6 @@ void shutdown_blkfront(struct blkfront_dev *dev) XenbusStateInitialising, err); goto close; } - err = NULL; state = xenbus_read_integer(path); while (err == NULL && (state < XenbusStateInitWait || state >= XenbusStateClosed)) err = xenbus_wait_for_state_change(path, &state, &dev->events); @@ -298,10 +297,10 @@ close: if (err) free(err); xenbus_unwatch_path_token(XBT_NIL, path, path); - snprintf(path, sizeof(path), "%s/ring-ref", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/event-channel", nodename); - xenbus_rm(XBT_NIL, path); + snprintf(nodename, sizeof(nodename), "%s/ring-ref", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); + xenbus_rm(XBT_NIL, nodename); if (!err) free_blkfront(dev); diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c index 54a5e67..39544b3 100644 --- a/extras/mini-os/fbfront.c +++ b/extras/mini-os/fbfront.c @@ -158,8 +158,8 @@ done: { XenbusState state; - char path[strlen(dev->backend) + 1 + 6 + 1]; - char frontpath[strlen(nodename) + 1 + 6 + 1]; + char path[strlen(dev->backend) + 1 + 5 + 1]; + char frontpath[strlen(nodename) + 1 + 5 + 1]; snprintf(path, sizeof(path), "%s/state", dev->backend); @@ -240,7 +240,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) XenbusState state; char path[strlen(dev->backend) + 1 + 5 + 1]; - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; + char nodename[strlen(dev->nodename) + 1 + 19 + 1]; printk("close kbd: backend at %s\n",dev->backend); @@ -272,7 +272,6 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) XenbusStateInitialising, err); goto close_kbdfront; } - err = NULL; state = xenbus_read_integer(path); while (err == NULL && (state < XenbusStateInitWait || state >= XenbusStateClosed)) err = xenbus_wait_for_state_change(path, &state, &dev->events); @@ -281,12 +280,12 @@ close_kbdfront: if (err) free(err); xenbus_unwatch_path_token(XBT_NIL, path, path); - snprintf(path, sizeof(path), "%s/page-ref", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/event-channel", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/request-abs-pointer", nodename); - xenbus_rm(XBT_NIL, path); + snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/request-abs-pointer", dev->nodename); + xenbus_rm(XBT_NIL, nodename); if (!err) free_kbdfront(dev); @@ -521,7 +520,7 @@ done: { XenbusState state; char path[strlen(dev->backend) + 1 + 14 + 1]; - char frontpath[strlen(nodename) + 1 + 6 + 1]; + char frontpath[strlen(nodename) + 1 + 5 + 1]; snprintf(path, sizeof(path), "%s/state", dev->backend); @@ -632,7 +631,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) XenbusState state; char path[strlen(dev->backend) + 1 + 5 + 1]; - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; + char nodename[strlen(dev->nodename) + 1 + 14 + 1]; printk("close fb: backend at %s\n",dev->backend); @@ -664,8 +663,6 @@ void shutdown_fbfront(struct fbfront_dev *dev) XenbusStateInitialising, err); goto close_fbfront; } - - err = NULL; state = xenbus_read_integer(path); while (err == NULL && (state < XenbusStateInitWait || state >= XenbusStateClosed)) err = xenbus_wait_for_state_change(path, &state, &dev->events); @@ -674,14 +671,14 @@ close_fbfront: if (err) free(err); xenbus_unwatch_path_token(XBT_NIL, path, path); - snprintf(path, sizeof(path), "%s/page-ref", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/event-channel", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/protocol", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/feature-update", nodename); - xenbus_rm(XBT_NIL, path); + snprintf(nodename, sizeof(nodename), "%s/page-ref", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/protocol", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/feature-update", dev->nodename); + xenbus_rm(XBT_NIL, nodename); if (!err) free_fbfront(dev); diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c index 6fa68a2..bafa84e 100644 --- a/extras/mini-os/netfront.c +++ b/extras/mini-os/netfront.c @@ -508,7 +508,7 @@ void shutdown_netfront(struct netfront_dev *dev) XenbusState state; char path[strlen(dev->backend) + 1 + 5 + 1]; - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; + char nodename[strlen(dev->nodename) + 1 + 15 + 1]; printk("close network: backend at %s\n",dev->backend); @@ -541,7 +541,6 @@ void shutdown_netfront(struct netfront_dev *dev) XenbusStateInitialising, err); goto close; } - err = NULL; state = xenbus_read_integer(path); while (err == NULL && (state < XenbusStateInitWait || state >= XenbusStateClosed)) err = xenbus_wait_for_state_change(path, &state, &dev->events); @@ -550,14 +549,14 @@ close: if (err) free(err); xenbus_unwatch_path_token(XBT_NIL, path, path); - snprintf(path, sizeof(path), "%s/tx-ring-ref", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/rx-ring-ref", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/event-channel", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/request-rx-copy", nodename); - xenbus_rm(XBT_NIL, path); + snprintf(nodename, sizeof(nodename), "%s/tx-ring-ref", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/rx-ring-ref", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/request-rx-copy", dev->nodename); + xenbus_rm(XBT_NIL, nodename); if (!err) free_netfront(dev); diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c index bbe21e0..23499dc 100644 --- a/extras/mini-os/pcifront.c +++ b/extras/mini-os/pcifront.c @@ -323,7 +323,7 @@ void shutdown_pcifront(struct pcifront_dev *dev) XenbusState state; char path[strlen(dev->backend) + 1 + 5 + 1]; - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; + char nodename[strlen(dev->nodename) + 1 + 13 + 1]; printk("close pci: backend at %s\n",dev->backend); @@ -355,7 +355,6 @@ void shutdown_pcifront(struct pcifront_dev *dev) XenbusStateInitialising, err); goto close_pcifront; } - err = NULL; state = xenbus_read_integer(path); while (err == NULL && (state < XenbusStateInitWait || state >= XenbusStateClosed)) err = xenbus_wait_for_state_change(path, &state, &dev->events); @@ -364,10 +363,10 @@ close_pcifront: if (err) free(err); xenbus_unwatch_path_token(XBT_NIL, path, path); - snprintf(path, sizeof(path), "%s/info-ref", nodename); - xenbus_rm(XBT_NIL, path); - snprintf(path, sizeof(path), "%s/event-channel", nodename); - xenbus_rm(XBT_NIL, path); + snprintf(nodename, sizeof(nodename), "%s/info-ref", dev->nodename); + xenbus_rm(XBT_NIL, nodename); + snprintf(nodename, sizeof(nodename), "%s/event-channel", dev->nodename); + xenbus_rm(XBT_NIL, nodename); if (!err) free_pcifront(dev); -- 1.7.9.5