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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).