[PATCH] xen-blkfront: prefer xenbus_scanf() over xenbus_gather()

[PATCH] xen-blkfront: prefer xenbus_scanf() over xenbus_gather()

[Jan Beulich]

... 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/block/xen-blkfront.c |   43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

--- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/block/xen-blkfront.c
+++ 4.7-rc6-prefer-xenbus_scanf/drivers/block/xen-blkfront.c
@@ -2208,10 +2208,9 @@ static void blkfront_setup_discard(struc
 		info->discard_granularity = discard_granularity;
 		info->discard_alignment = discard_alignment;
 	}
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-		    "discard-secure", "%d", &discard_secure,
-		    NULL);
-	if (!err)
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "discard-secure", "%u", &discard_secure);
+	if (err > 0)
 		info->feature_secdiscard = !!discard_secure;
 }
 
@@ -2310,9 +2309,8 @@ static void blkfront_gather_backend_feat
 
 	info->feature_flush = 0;
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-barrier", "%d", &barrier,
-			NULL);
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-barrier", "%d", &barrier);
 
 	/*
 	 * If there's no "feature-barrier" defined, then it means
@@ -2321,38 +2319,35 @@ static void blkfront_gather_backend_feat
 	 *
 	 * If there are barriers, then we use flush.
 	 */
-	if (!err && barrier)
+	if (err > 0 && barrier)
 		info->feature_flush = REQ_FLUSH | REQ_FUA;
 	/*
 	 * And if there is "feature-flush-cache" use that above
 	 * barriers.
 	 */
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-flush-cache", "%d", &flush,
-			NULL);
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-flush-cache", "%d", &flush);
 
-	if (!err && flush)
+	if (err > 0 && flush)
 		info->feature_flush = REQ_FLUSH;
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-discard", "%d", &discard,
-			NULL);
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-discard", "%d", &discard);
 
-	if (!err && discard)
+	if (err > 0 && discard)
 		blkfront_setup_discard(info);
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			"feature-persistent", "%u", &persistent,
-			NULL);
-	if (err)
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-persistent", "%d", &persistent);
+	if (err <= 0)
 		info->feature_persistent = 0;
 	else
 		info->feature_persistent = persistent;
 
-	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
-			    "feature-max-indirect-segments", "%u", &indirect_segments,
-			    NULL);
-	if (err)
+	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
+			   "feature-max-indirect-segments", "%u",
+			   &indirect_segments);
+	if (err <= 0)
 		info->max_indirect_segments = 0;
 	else
 		info->max_indirect_segments = min(indirect_segments,

Re: [PATCH] xen-blkfront: prefer xenbus_scanf() over xenbus_gather()

[Roger Pau Monne]

On Thu, Jul 07, 2016 at 02:05:46AM -0600, 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.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
>  drivers/block/xen-blkfront.c |   43 +++++++++++++++++++------------------------
>  1 file changed, 19 insertions(+), 24 deletions(-)
> 
> --- 4.7-rc6-prefer-xenbus_scanf.orig/drivers/block/xen-blkfront.c
> +++ 4.7-rc6-prefer-xenbus_scanf/drivers/block/xen-blkfront.c
> @@ -2208,10 +2208,9 @@ static void blkfront_setup_discard(struc
>  		info->discard_granularity = discard_granularity;
>  		info->discard_alignment = discard_alignment;
>  	}
> -	err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
> -		    "discard-secure", "%d", &discard_secure,
> -		    NULL);
> -	if (!err)
> +	err = xenbus_scanf(XBT_NIL, info->xbdev->otherend,
> +			   "discard-secure", "%u", &discard_secure);
> +	if (err > 0)

I would prefer an explicit err == 1 check (here and elsewhere), but I'm not 
going to block this patch because of that.

Roger.