Message ID | 577E279D02000078000FBEBF@prv-mh.provo.novell.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 07/07/16 08:57, Jan Beulich wrote: > ... for single items being collected: It is more typesafe (as the > compiler can check format string and to-be-written-to variable match) > and requires one less parameter to be passed. Do not split commit messages between the subject and the body in this way. The body should be a complete description of the commit, independent of the subject. It's annoying to find only half a sentence in the body and have to go re-read the subject again. e.g., "xen-netback: prefer xenbus_scanf() over xenbus_gather() xenbus_scanf() is preferred over xenbus_gather() because: a) the compiler can check the types match the format; and b) one less parameter is needed. Use xenbus_scanf() instead of xenbus_gather() when reading only a single key." I think almost all of your commit messages suffer from this, please redo them all. > --- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/net/xen-netback/xenbus.c > +++ 4.7-rc6-prefer-xenbus_scanf/drivers/net/xen-netback/xenbus.c > @@ -842,16 +842,16 @@ static int connect_ctrl_ring(struct back > unsigned int evtchn; > int err; > > - err = xenbus_gather(XBT_NIL, dev->otherend, > - "ctrl-ring-ref", "%u", &val, NULL); > - if (err) > + err = xenbus_scanf(XBT_NIL, dev->otherend, > + "ctrl-ring-ref", "%u", &val); > + if (err <= 0) < 0 David
>>> On 07.07.16 at 11:55, <david.vrabel@citrix.com> wrote: > On 07/07/16 08:57, Jan Beulich wrote: >> --- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/net/xen-netback/xenbus.c >> +++ 4.7-rc6-prefer-xenbus_scanf/drivers/net/xen-netback/xenbus.c >> @@ -842,16 +842,16 @@ static int connect_ctrl_ring(struct back >> unsigned int evtchn; >> int err; >> >> - err = xenbus_gather(XBT_NIL, dev->otherend, >> - "ctrl-ring-ref", "%u", &val, NULL); >> - if (err) >> + err = xenbus_scanf(XBT_NIL, dev->otherend, >> + "ctrl-ring-ref", "%u", &val); >> + if (err <= 0) > > < 0 I disagree, as said elsewhere. If you feel like adjusting existing instances is unnecessary, then drop those patches on the floor. But I don't like to submit new uses with such a bogus check. Jan
--- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/net/xen-netback/xenbus.c +++ 4.7-rc6-prefer-xenbus_scanf/drivers/net/xen-netback/xenbus.c @@ -842,16 +842,16 @@ static int connect_ctrl_ring(struct back unsigned int evtchn; int err; - err = xenbus_gather(XBT_NIL, dev->otherend, - "ctrl-ring-ref", "%u", &val, NULL); - if (err) + err = xenbus_scanf(XBT_NIL, dev->otherend, + "ctrl-ring-ref", "%u", &val); + if (err <= 0) goto done; /* The frontend does not have a control ring */ ring_ref = val; - err = xenbus_gather(XBT_NIL, dev->otherend, - "event-channel-ctrl", "%u", &val, NULL); - if (err) { + err = xenbus_scanf(XBT_NIL, dev->otherend, + "event-channel-ctrl", "%u", &val); + if (err <= 0) { xenbus_dev_fatal(dev, err, "reading %s/event-channel-ctrl", dev->otherend); @@ -872,7 +872,7 @@ done: return 0; fail: - return err; + return err ?: -ENODATA; } static void connect(struct backend_info *be)
... for single items being collected: It is more typesafe (as the compiler can check format string and to-be-written-to variable match) and requires one less parameter to be passed. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- drivers/net/xen-netback/xenbus.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)