WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] linux-2.6.18: xenbus_gather()/xenbus_scanf() usage a

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] linux-2.6.18: xenbus_gather()/xenbus_scanf() usage adjustments
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Fri, 17 Jun 2011 16:37:34 +0100
Delivery-date: Fri, 17 Jun 2011 08:38:25 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
- don't use xenbus_gather() for just a single, non-string item (as it
  doesn't do format checking on its inputs)
- grant references don't need to be parsed as "long", since grant_ref_t
  is a typedef of uint32_t
- in the frontend protocol determination logic in blkback and blktap,
  don't parse into a local buffer - use the allocated string directly

Signed-off-by: Jan Beulcih <jbeulich@xxxxxxxxxx>

--- a/drivers/xen/blkback/xenbus.c
+++ b/drivers/xen/blkback/xenbus.c
@@ -485,14 +485,13 @@ again:
 static int connect_ring(struct backend_info *be)
 {
        struct xenbus_device *dev = be->dev;
-       unsigned long ring_ref;
-       unsigned int evtchn;
-       char protocol[64] = "";
+       unsigned int ring_ref, evtchn;
+       char *protocol;
        int err;
 
        DPRINTK("%s", dev->otherend);
 
-       err = xenbus_gather(XBT_NIL, dev->otherend, "ring-ref", "%lu", 
&ring_ref,
+       err = xenbus_gather(XBT_NIL, dev->otherend, "ring-ref", "%u", &ring_ref,
                            "event-channel", "%u", &evtchn, NULL);
        if (err) {
                xenbus_dev_fatal(dev, err,
@@ -503,9 +502,9 @@ static int connect_ring(struct backend_i
 
        be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
        err = xenbus_gather(XBT_NIL, dev->otherend, "protocol",
-                           "%63s", protocol, NULL);
+                           NULL, &protocol, NULL);
        if (err)
-               strcpy(protocol, "unspecified, assuming native");
+               protocol = NULL;
        else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE))
                be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
        else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32))
@@ -514,16 +513,19 @@ static int connect_ring(struct backend_i
                be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64;
        else {
                xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
+               kfree(protocol);
                return -1;
        }
        printk(KERN_INFO
-              "blkback: ring-ref %ld, event-channel %d, protocol %d (%s)\n",
-              ring_ref, evtchn, be->blkif->blk_protocol, protocol);
+              "blkback: ring-ref %u, event-channel %u, protocol %d (%s)\n",
+              ring_ref, evtchn, be->blkif->blk_protocol,
+              protocol ?: "unspecified, assuming native");
+       kfree(protocol);
 
        /* Map the shared frame, irq etc. */
        err = blkif_map(be->blkif, ring_ref, evtchn);
        if (err) {
-               xenbus_dev_fatal(dev, err, "mapping ring-ref %lu port %u",
+               xenbus_dev_fatal(dev, err, "mapping ring-ref %u port %u",
                                 ring_ref, evtchn);
                return err;
        }
--- a/drivers/xen/blkfront/blkfront.c
+++ b/drivers/xen/blkfront/blkfront.c
@@ -334,7 +334,7 @@ static void connect(struct blkfront_info
                 */
                err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
                                   "sectors", "%Lu", &sectors);
-               if (XENBUS_EXIST_ERR(err))
+               if (err != 1)
                        return;
                printk(KERN_INFO "Setting capacity to %Lu\n",
                       sectors);
@@ -359,10 +359,9 @@ static void connect(struct blkfront_info
                return;
        }
 
-       err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-                           "feature-barrier", "%d", &info->feature_barrier,
-                           NULL);
-       if (err)
+       err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+                          "feature-barrier", "%d", &info->feature_barrier);
+       if (err <= 0)
                info->feature_barrier = 0;
 
        err = xlvbd_add(sectors, info->vdevice, binfo, sector_size, info);
--- a/drivers/xen/blktap/xenbus.c
+++ b/drivers/xen/blktap/xenbus.c
@@ -319,7 +319,7 @@ static void tap_backend_changed(struct x
         * and disk info to xenstore
         */
        err = xenbus_gather(XBT_NIL, dev->nodename, "info", "%lu", &info, 
-                           NULL);
+                           "sectors", "%Lu", &be->blkif->sectors, NULL);
        if (XENBUS_EXIST_ERR(err))
                return;
        if (err) {
@@ -329,9 +329,6 @@ static void tap_backend_changed(struct x
 
        DPRINTK("Userspace update on disk info, %lu\n",info);
 
-       err = xenbus_gather(XBT_NIL, dev->nodename, "sectors", "%llu", 
-                           &be->blkif->sectors, NULL);
-
        /* Associate tap dev with domid*/
        be->blkif->dev_num = dom_to_devid(be->blkif->domid, be->xenbus_id, 
                                          be->blkif);
@@ -436,14 +433,13 @@ again:
 static int connect_ring(struct backend_info *be)
 {
        struct xenbus_device *dev = be->dev;
-       unsigned long ring_ref;
-       unsigned int evtchn;
-       char protocol[64];
+       unsigned int ring_ref, evtchn;
+       char *protocol;
        int err;
 
        DPRINTK("%s\n", dev->otherend);
 
-       err = xenbus_gather(XBT_NIL, dev->otherend, "ring-ref", "%lu", 
+       err = xenbus_gather(XBT_NIL, dev->otherend, "ring-ref", "%u",
                            &ring_ref, "event-channel", "%u", &evtchn, NULL);
        if (err) {
                xenbus_dev_fatal(dev, err,
@@ -454,9 +450,9 @@ static int connect_ring(struct backend_i
 
        be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
        err = xenbus_gather(XBT_NIL, dev->otherend, "protocol",
-                           "%63s", protocol, NULL);
+                           NULL, &protocol, NULL);
        if (err)
-               strcpy(protocol, "unspecified, assuming native");
+               protocol = NULL;
        else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_NATIVE))
                be->blkif->blk_protocol = BLKIF_PROTOCOL_NATIVE;
        else if (0 == strcmp(protocol, XEN_IO_PROTO_ABI_X86_32))
@@ -465,16 +461,19 @@ static int connect_ring(struct backend_i
                be->blkif->blk_protocol = BLKIF_PROTOCOL_X86_64;
        else {
                xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol);
+               kfree(protocol);
                return -1;
        }
        printk(KERN_INFO
-              "blktap: ring-ref %ld, event-channel %d, protocol %d (%s)\n",
-              ring_ref, evtchn, be->blkif->blk_protocol, protocol);
+              "blktap: ring-ref %u, event-channel %u, protocol %d (%s)\n",
+              ring_ref, evtchn, be->blkif->blk_protocol,
+              protocol ?: "unspecified, assuming native");
+       kfree(protocol);
 
        /* Map the shared frame, irq etc. */
        err = tap_blkif_map(be->blkif, dev, ring_ref, evtchn);
        if (err) {
-               xenbus_dev_fatal(dev, err, "mapping ring-ref %lu port %u",
+               xenbus_dev_fatal(dev, err, "mapping ring-ref %u port %u",
                                 ring_ref, evtchn);
                return err;
        } 
--- a/drivers/xen/core/reboot.c
+++ b/drivers/xen/core/reboot.c
@@ -215,7 +215,7 @@ static void sysrq_handler(struct xenbus_
        err = xenbus_transaction_start(&xbt);
        if (err)
                return;
-       if (!xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key)) {
+       if (xenbus_scanf(xbt, "control", "sysrq", "%c", &sysrq_key) <= 0) {
                printk(KERN_ERR "Unable to read sysrq code in "
                       "control/sysrq\n");
                xenbus_transaction_end(xbt, 1);
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -358,7 +358,7 @@ static int connect_rings(struct backend_
 {
        netif_t *netif = be->netif;
        struct xenbus_device *dev = be->dev;
-       unsigned long tx_ring_ref, rx_ring_ref;
+       unsigned int tx_ring_ref, rx_ring_ref;
        unsigned int evtchn, rx_copy;
        int err;
        int val;
@@ -366,8 +366,8 @@ static int connect_rings(struct backend_
        DPRINTK("");
 
        err = xenbus_gather(XBT_NIL, dev->otherend,
-                           "tx-ring-ref", "%lu", &tx_ring_ref,
-                           "rx-ring-ref", "%lu", &rx_ring_ref,
+                           "tx-ring-ref", "%u", &tx_ring_ref,
+                           "rx-ring-ref", "%u", &rx_ring_ref,
                            "event-channel", "%u", &evtchn, NULL);
        if (err) {
                xenbus_dev_fatal(dev, err,
@@ -421,7 +421,7 @@ static int connect_rings(struct backend_
        err = netif_map(be, tx_ring_ref, rx_ring_ref, evtchn);
        if (err) {
                xenbus_dev_fatal(dev, err,
-                                "mapping shared-frames %lu/%lu port %u",
+                                "mapping shared-frames %u/%u port %u",
                                 tx_ring_ref, rx_ring_ref, evtchn);
                return err;
        }
--- a/drivers/xen/scsiback/xenbus.c
+++ b/drivers/xen/scsiback/xenbus.c
@@ -60,13 +60,12 @@ static int __vscsiif_name(struct backend
 static int scsiback_map(struct backend_info *be)
 {
        struct xenbus_device *dev = be->dev;
-       unsigned long ring_ref;
-       unsigned int evtchn;
+       unsigned int ring_ref, evtchn;
        int err;
        char name[TASK_COMM_LEN];
 
        err = xenbus_gather(XBT_NIL, dev->otherend,
-                       "ring-ref", "%lu", &ring_ref,
+                       "ring-ref", "%u", &ring_ref,
                        "event-channel", "%u", &evtchn, NULL);
        if (err) {
                xenbus_dev_fatal(dev, err, "reading %s ring", dev->otherend);
--- a/drivers/xen/tpmback/xenbus.c
+++ b/drivers/xen/tpmback/xenbus.c
@@ -210,12 +210,11 @@ abort:
 static int connect_ring(struct backend_info *be)
 {
        struct xenbus_device *dev = be->dev;
-       unsigned long ring_ref;
-       unsigned int evtchn;
+       unsigned int ring_ref, evtchn;
        int err;
 
        err = xenbus_gather(XBT_NIL, dev->otherend,
-                           "ring-ref", "%lu", &ring_ref,
+                           "ring-ref", "%u", &ring_ref,
                            "event-channel", "%u", &evtchn, NULL);
        if (err) {
                xenbus_dev_error(dev, err,
@@ -238,7 +237,7 @@ static int connect_ring(struct backend_i
                err = tpmif_map(be->tpmif, ring_ref, evtchn);
                if (err) {
                        xenbus_dev_error(dev, err,
-                                        "mapping shared-frame %lu port %u",
+                                        "mapping shared-frame %u port %u",
                                         ring_ref, evtchn);
                        return err;
                }
--- a/drivers/xen/usbback/xenbus.c
+++ b/drivers/xen/usbback/xenbus.c
@@ -226,14 +226,12 @@ fail:
 static int connect_rings(usbif_t *usbif)
 {
        struct xenbus_device *dev = usbif->xbdev;
-       unsigned long urb_ring_ref;
-       unsigned long conn_ring_ref;
-       unsigned int evtchn;
+       unsigned int urb_ring_ref, conn_ring_ref, evtchn;
        int err;
 
        err = xenbus_gather(XBT_NIL, dev->otherend,
-                           "urb-ring-ref", "%lu", &urb_ring_ref,
-                           "conn-ring-ref", "%lu", &conn_ring_ref,
+                           "urb-ring-ref", "%u", &urb_ring_ref,
+                           "conn-ring-ref", "%u", &conn_ring_ref,
                            "event-channel", "%u", &evtchn, NULL);
        if (err) {
                xenbus_dev_fatal(dev, err,
@@ -242,13 +240,14 @@ static int connect_rings(usbif_t *usbif)
                return err;
        }
 
-       printk("usbback: urb-ring-ref %ld, conn-ring-ref %ld, event-channel 
%d\n",
+       printk("usbback: urb-ring-ref %u, conn-ring-ref %u,"
+              " event-channel %u\n",
               urb_ring_ref, conn_ring_ref, evtchn);
 
        err = usbif_map(usbif, urb_ring_ref, conn_ring_ref, evtchn);
        if (err) {
                xenbus_dev_fatal(dev, err,
-                               "mapping urb-ring-ref %lu conn-ring-ref %lu 
port %u",
+                               "mapping urb-ring-ref %u conn-ring-ref %u port 
%u",
                                urb_ring_ref, conn_ring_ref, evtchn);
                return err;
        }
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -276,9 +276,9 @@ EXPORT_SYMBOL_GPL(xenbus_free_evtchn);
 
 enum xenbus_state xenbus_read_driver_state(const char *path)
 {
-       enum xenbus_state result;
-       int err = xenbus_gather(XBT_NIL, path, "state", "%d", &result, NULL);
-       if (err)
+       int result;
+
+       if (xenbus_scanf(XBT_NIL, path, "state", "%d", &result) != 1)
                result = XenbusStateUnknown;
 
        return result;
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -865,7 +865,8 @@ static int be_state;
 
 static void xenbus_reset_state_changed(struct xenbus_watch *w, const char **v, 
unsigned int l)
 {
-       xenbus_scanf(XBT_NIL, v[XS_WATCH_PATH], "", "%i", &be_state);
+       if (xenbus_scanf(XBT_NIL, v[XS_WATCH_PATH], "", "%i", &be_state) != 1)
+               be_state = XenbusStateUnknown;
        printk(KERN_INFO "XENBUS: %s %s\n", v[XS_WATCH_PATH], 
xenbus_strstate(be_state));
        wake_up(&be_state_wq);
 }


Attachment: xen-xenbus-scanf-gather.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] linux-2.6.18: xenbus_gather()/xenbus_scanf() usage adjustments, Jan Beulich <=