All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <boaz@plexistor.com>
To: Brian Norris <computersforpeace@gmail.com>, Jens Axboe <axboe@fb.com>
Cc: linux-kernel@vger.kernel.org, Christoph Hellwig <hch@infradead.org>
Subject: Re: [RFC] block: remove never-modified global variable
Date: Tue, 19 May 2015 10:34:56 +0300	[thread overview]
Message-ID: <555AE7A0.9040505@plexistor.com> (raw)
In-Reply-To: <555AE3F0.5010109@plexistor.com>

On 05/19/2015 10:19 AM, Boaz Harrosh wrote:
<>
> But specially now that you are unconditionally printing it. It is better
> to just combine the two statements. See suggested patch below:
> 

Actually we can even do better:

diff --git a/block/partitions/check.c b/block/partitions/check.c
index 513c601..9bea23d 100644
--- a/block/partitions/check.c
+++ b/block/partitions/check.c
@@ -160,35 +160,31 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 
 	i = res = err = 0;
 	while (!res && check_part[i]) {
 		memset(state->parts, 0, state->limit * sizeof(state->parts[0]));
 		res = check_part[i++](state);
 		if (res < 0) {
 			/* We have hit an I/O error which we don't report now.
 		 	* But record it, and let the others do their job.
 		 	*/
 			err = res;
 			res = 0;
 		}
 
 	}

boaz> When we exit the loop res can only be (res >= 0)

 	if (res > 0) {
 		printk(KERN_INFO "%s", state->pp_buf);
 
 		free_page((unsigned long)state->pp_buf);
 		return state;

boaz> When bigger we exit here

 	}
 	if (state->access_beyond_eod)
 		err = -ENOSPC;

boaz> So by this stage res can only be == 0. So we should just do:

 	if (err)
	/* The partition is unrecognized. So report I/O errors if there were any */
-		res = err;
-	if (res) {
-		strlcat(state->pp_buf,
-			" unable to read partition table\n", PAGE_SIZE);
-		printk(KERN_INFO "%s", state->pp_buf);
-	}
+		printk(KERN_INFO "%s unable to read partition table\n",
+		       state->pp_buf);
 
 	free_page((unsigned long)state->pp_buf);
 	free_partitions(state);
 	return ERR_PTR(res);
 }

----
Here is a proper diff:
----
diff --git a/block/partitions/check.c b/block/partitions/check.c
index 513c601..9bea23d 100644
--- a/block/partitions/check.c
+++ b/block/partitions/check.c
@@ -181,12 +181,8 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 		err = -ENOSPC;
 	if (err)
 	/* The partition is unrecognized. So report I/O errors if there were any */
-		res = err;
-	if (res) {
-		strlcat(state->pp_buf,
-			" unable to read partition table\n", PAGE_SIZE);
-		printk(KERN_INFO "%s", state->pp_buf);
-	}
+		printk(KERN_INFO "%s unable to read partition table\n",
+		       state->pp_buf);
 
 	free_page((unsigned long)state->pp_buf);
 	free_partitions(state);

Thanks
Boaz

  reply	other threads:[~2015-05-19  7:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-18 23:08 [RFC] block: remove never-modified global variable Brian Norris
2015-05-19  7:19 ` Boaz Harrosh
2015-05-19  7:34   ` Boaz Harrosh [this message]
2015-05-19 17:55     ` Brian Norris
2015-05-19 17:58       ` Brian Norris
2015-05-20 11:18         ` Boaz Harrosh
2015-05-21  0:32           ` [PATCH v2] " Brian Norris

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=555AE7A0.9040505@plexistor.com \
    --to=boaz@plexistor.com \
    --cc=axboe@fb.com \
    --cc=computersforpeace@gmail.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.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.