[PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[Atsushi Nemoto]

On "partition is out of reach" path, i.e. slave's offset was bigger
than the master's size, slave's erasesize will not be calculated and
it leads division by zero on following boundary checking.  This patch
makes calculation of the slave's erasesize more robust.

This patch also contains some cleanups.  Do not shadow symbol 'i',
fold long lines, etc.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
 drivers/mtd/mtdpart.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 07c7011..ff9ea3a 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -445,17 +445,22 @@ int add_mtd_partitions(struct mtd_info *master,
 		}
 		if (master->numeraseregions>1) {
 			/* Deal with variable erase size stuff */
-			int i;
-			struct mtd_erase_region_info *regions = master->eraseregions;
-
-			/* Find the first erase regions which is part of this partition. */
-			for (i=0; i < master->numeraseregions && slave->offset >= regions[i].offset; i++)
+			int j, num = master->numeraseregions;
+			struct mtd_erase_region_info *r = master->eraseregions;
+			/*
+			 * Find the first erase region which is part
+			 * of this partition.
+			 */
+			for (j = 0; j < num && slave->offset >= r[j].offset;
+				j++)
 				;
-
-			for (i--; i < master->numeraseregions && slave->offset + slave->mtd.size > regions[i].offset; i++) {
-				if (slave->mtd.erasesize < regions[i].erasesize) {
-					slave->mtd.erasesize = regions[i].erasesize;
-				}
+			j--;
+			slave->mtd.erasesize = r[j].erasesize;
+			for (; j < num &&
+				slave->offset + slave->mtd.size > r[j].offset;
+				j++) {
+				if (slave->mtd.erasesize < r[j].erasesize)
+					slave->mtd.erasesize = r[j].erasesize;
 			}
 		} else {
 			/* Single erase size */

Re: [PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[Jörn Engel]

On Sat, 14 June 2008 23:45:40 +0900, Atsushi Nemoto wrote:
> 
> On "partition is out of reach" path, i.e. slave's offset was bigger
> than the master's size, slave's erasesize will not be calculated and
> it leads division by zero on following boundary checking.  This patch
> makes calculation of the slave's erasesize more robust.
> 
> This patch also contains some cleanups.  Do not shadow symbol 'i',
> fold long lines, etc.

I personally consider the original long "for (...)" lines slightly
better (or rather less bad) than your replacement.  Mixing cleanups
with bugfixes is often a bad idea.

Another reason is that I still cannot see the bugfix among all the
changes. *scratches head*  Ah, here it is.  *scratches some more*  And
it even appears to be correct.

Can you resend as two patches?

Jörn

-- 
A victorious army first wins and then seeks battle.
-- Sun Tzu

Re: [PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[Atsushi Nemoto]

On Mon, 16 Jun 2008 10:33:56 +0200, Jörn Engel <joern@logfs.org> wrote:
> > On "partition is out of reach" path, i.e. slave's offset was bigger
> > than the master's size, slave's erasesize will not be calculated and
> > it leads division by zero on following boundary checking.  This patch
> > makes calculation of the slave's erasesize more robust.
> > 
> > This patch also contains some cleanups.  Do not shadow symbol 'i',
> > fold long lines, etc.
> 
> I personally consider the original long "for (...)" lines slightly
> better (or rather less bad) than your replacement.  Mixing cleanups
> with bugfixes is often a bad idea.
> 
> Another reason is that I still cannot see the bugfix among all the
> changes. *scratches head*  Ah, here it is.  *scratches some more*  And
> it even appears to be correct.
> 
> Can you resend as two patches?

OK, I will send two patches --- one for the bugfix and one for sparse
warning.  Remaining cleanup ("Folding long lines, etc.") was just for
checkpatch.pl and if these new patches can be accepted, I gladly keep
code as is.  The mtdpart.c has many long lines anyway. :-)

Thank you for review.

---
Atsushi Nemoto

Re: [PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[Jörn Engel]

On Mon, 16 June 2008 23:30:39 +0900, Atsushi Nemoto wrote:
> 
> OK, I will send two patches --- one for the bugfix and one for sparse
> warning.  Remaining cleanup ("Folding long lines, etc.") was just for
> checkpatch.pl and if these new patches can be accepted, I gladly keep
> code as is.  The mtdpart.c has many long lines anyway. :-)

David seems to like long lines.  Sometimes even for good reasons.

Jörn

-- 
Audacity augments courage; hesitation, fear.
-- Publilius Syrus

Re: [PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[David Woodhouse]

On Mon, 2008-06-16 at 16:57 +0200, Jörn Engel wrote:
> David seems to like long lines.  Sometimes even for good reasons.

Sometimes I do, yes. But at a quick glance I don't think I would argue
that we should keep _those_ ones. They could be wrapped without
detriment.

-- 
dwmw2

Re: [PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[Atsushi Nemoto]

On Mon, 16 Jun 2008 16:03:54 +0100, David Woodhouse <dwmw2@infradead.org> wrote:
> > David seems to like long lines.  Sometimes even for good reasons.
> 
> Sometimes I do, yes. But at a quick glance I don't think I would argue
> that we should keep _those_ ones. They could be wrapped without
> detriment.

Then, should I revise these patches with holding long lines?

> Subject: [PATCH 1/2] mtdpart: Avoid divide-by-zero on out-of-reach path
> Subject: [PATCH 2/2] mtdpart: Do not shadow symbol 'i'

---
Atsushi Nemoto

Re: [PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[David Woodhouse]

On Tue, 2008-06-17 at 23:09 +0900, Atsushi Nemoto wrote:
> On Mon, 16 Jun 2008 16:03:54 +0100, David Woodhouse <dwmw2@infradead.org> wrote:
> > > David seems to like long lines.  Sometimes even for good reasons.
> > 
> > Sometimes I do, yes. But at a quick glance I don't think I would argue
> > that we should keep _those_ ones. They could be wrapped without
> > detriment.
> 
> Then, should I revise these patches with holding long lines?

As Jörn says, it's probably better to do things one logical step at a
time. Why not follow up with a third patch which wraps the long lines in
the whole file (as long as that doesn't make it harder to read).

-- 
dwmw2

Re: [PATCH RESEND] mtdpart: Avoid divide-by-zero on out-of-reach path

[Atsushi Nemoto]

On Tue, 17 Jun 2008 15:10:19 +0100, David Woodhouse <dwmw2@infradead.org> wrote:
> > Then, should I revise these patches with holding long lines?
> 
> As Jörn says, it's probably better to do things one logical step at a
> time. Why not follow up with a third patch which wraps the long lines in
> the whole file (as long as that doesn't make it harder to read).

OK, I will send a cleanup patch soon after these two patches
stabilized.  It seems Jörn still not comfortable with current shape :)

---
Atsushi Nemoto