From: Matt Wilson <msw@linux.com>
To: xen-devel@lists.xenproject.org
Cc: Ben Cressey <bcressey@amazon.com>,
Samuel Thibault <samuel.thibault@ens-lyon.org>,
Matt Wilson <msw@amazon.com>,
Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: [PATCH] minios: Fix xenbus_rm() calls in frontend drivers
Date: Wed, 4 Sep 2013 17:25:20 -0700 [thread overview]
Message-ID: <1378340720-8653-1-git-send-email-msw@linux.com> (raw)
From: Ben Cressey <bcressey@amazon.com>
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 <bcressey@amazon.com>
Reviewed-by: Matt Wilson <msw@amazon.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Matt Wilson <msw@amazon.com>
---
| 11 +++++------
| 41 +++++++++++++++++++----------------------
| 19 +++++++++----------
| 11 +++++------
4 files changed, 38 insertions(+), 44 deletions(-)
--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);
--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);
--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);
--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
next reply other threads:[~2013-09-05 0:25 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-05 0:25 Matt Wilson [this message]
2013-09-05 9:17 ` [PATCH] minios: Fix xenbus_rm() calls in frontend drivers Ian Campbell
2013-09-05 18:06 ` Matt Wilson
2013-09-06 9:00 ` Ian Campbell
2013-09-06 16:24 ` Matt Wilson
2013-09-06 16:53 ` Ian Campbell
2013-09-06 19:52 ` [PATCH v2 0/4] minios: various cleanups and fixes Matt Wilson
2013-09-06 19:52 ` [PATCH v2 1/4] minios: correct char array allocation for xenbus paths Matt Wilson
2013-09-09 14:00 ` Ian Campbell
2013-09-09 18:53 ` Samuel Thibault
2013-09-06 19:52 ` [PATCH v2 2/4] minios: clean up allocation of char arrays used " Matt Wilson
2013-09-09 14:02 ` Ian Campbell
2013-09-06 19:52 ` [PATCH v2 3/4] minios: clean up unneeded "err = NULL" in frontend drivers Matt Wilson
2013-09-06 19:52 ` [PATCH v2 4/4] minios: fix xenbus_rm() calls " Matt Wilson
2013-09-09 14:04 ` [PATCH v2 0/4] minios: various cleanups and fixes Ian Campbell
2013-09-10 10:48 ` Ian Campbell
2013-09-05 18:17 ` [PATCH] minios: Fix xenbus_rm() calls in frontend drivers Samuel Thibault
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1378340720-8653-1-git-send-email-msw@linux.com \
--to=msw@linux.com \
--cc=bcressey@amazon.com \
--cc=msw@amazon.com \
--cc=samuel.thibault@ens-lyon.org \
--cc=stefano.stabellini@eu.citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.