[BUG] mtd-utils: nandwrite: invalid erase after page write failure

[BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Fryar, Jeff]

mtd-utils nandwrite.c: After a page write failure, the calculation
of the block number to erase is incorrect.  The erase block size is 
being passed as the erase block number in the call to mtd_erase().

Jeff Fryar <jeff.fryar@hp.com>


--- mtd-utils-f1fe865/nandwrite.c.ORIG  2011-07-20 00:28:52.000000000 -0500
+++ mtd-utils-f1fe865/nandwrite.c       2011-08-11 09:19:12.000000000 -0500
@@ -659,7 +659,7 @@
                        fprintf(stderr, "Erasing failed write from %#08llx to %#08llx\n",
                                blockstart, blockstart + ebsize_aligned - 1);
                        for (i = blockstart; i < blockstart + ebsize_aligned; i += mtd.eb_size) {
-                               if (mtd_erase(mtd_desc, &mtd, fd, mtd.eb_size)) {
+                               if (mtd_erase(mtd_desc, &mtd, fd, (i/mtd.eb_size))) {
                                        int errno_tmp = errno;
                                        sys_errmsg("%s: MTD Erase failure", mtd_device);
                                        if (errno_tmp != EIO) {

Re: [BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Mike Frysinger]

On Thu, Aug 11, 2011 at 11:30, Fryar, Jeff wrote:
> +                               if (mtd_erase(mtd_desc, &mtd, fd, (i/mtd.eb_size))) {

we dont need to conserve whitespace.  please use spaces around "/" operators.
-mike

RE: [BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Fryar, Jeff]

Suggested fix (now with more whitespace)  :)

Jeff Fryar <jeff.fryar at hp.com>


--- mtd-utils-f1fe865/nandwrite.c.ORIG  2011-07-20 00:28:52.000000000 -0500
+++ mtd-utils-f1fe865/nandwrite.c       2011-08-11 09:19:12.000000000 -0500
@@ -659,7 +659,7 @@
                        fprintf(stderr, "Erasing failed write from %#08llx to %#08llx\n",
                                blockstart, blockstart + ebsize_aligned - 1);
                        for (i = blockstart; i < blockstart + ebsize_aligned; i += mtd.eb_size) {
-                               if (mtd_erase(mtd_desc, &mtd, fd, mtd.eb_size)) {
+                               if (mtd_erase(mtd_desc, &mtd, fd, (i / mtd.eb_size))) {
                                        int errno_tmp = errno;
                                        sys_errmsg("%s: MTD Erase failure", mtd_device);
                                        if (errno_tmp != EIO) {

Re: [BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Artem Bityutskiy]

On Thu, 2011-08-11 at 11:39 -0400, Mike Frysinger wrote:
> On Thu, Aug 11, 2011 at 11:30, Fryar, Jeff wrote:
> > +                               if (mtd_erase(mtd_desc, &mtd, fd, (i/mtd.eb_size))) {
> 
> we dont need to conserve whitespace.  please use spaces around "/" operators.

Yeah, I added white-spaces and removed unneeded brackets.

-- 
Best Regards,
Artem Bityutskiy

Re: [BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Artem Bityutskiy]

On Thu, 2011-08-11 at 16:30 +0100, Fryar, Jeff wrote:
> mtd-utils nandwrite.c: After a page write failure, the calculation
> of the block number to erase is incorrect.  The erase block size is 
> being passed as the erase block number in the call to mtd_erase().
> 
> Jeff Fryar <jeff.fryar@hp.com>
> 
> 
> --- mtd-utils-f1fe865/nandwrite.c.ORIG  2011-07-20 00:28:52.000000000 -0500
> +++ mtd-utils-f1fe865/nandwrite.c       2011-08-11 09:19:12.000000000 -0500
> @@ -659,7 +659,7 @@
>                         fprintf(stderr, "Erasing failed write from %#08llx to %#08llx\n",
>                                 blockstart, blockstart + ebsize_aligned - 1);
>                         for (i = blockstart; i < blockstart + ebsize_aligned; i += mtd.eb_size) {
> -                               if (mtd_erase(mtd_desc, &mtd, fd, mtd.eb_size)) {
> +                               if (mtd_erase(mtd_desc, &mtd, fd, (i/mtd.eb_size))) {

Looks like correct fix, thanks! Your patch is not very well formatted,
but I've amended it and applied to mtd-utils, thanks!

-- 
Best Regards,
Artem Bityutskiy

Re: [BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Brian Norris]

On Fri, Aug 19, 2011 at 10:02 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Thu, 2011-08-11 at 16:30 +0100, Fryar, Jeff wrote:
>> mtd-utils nandwrite.c: After a page write failure, the calculation
>> of the block number to erase is incorrect.  The erase block size is
>> being passed as the erase block number in the call to mtd_erase().
>
> Looks like correct fix, thanks! Your patch is not very well formatted,
> but I've amended it and applied to mtd-utils, thanks!

Oops, my bad on the bug. I didn't ever get around to using this
feature when I updated nandwrite a while back. Thanks for catching it.

BTW, are you going to rebase or merge the v1.4.6 tag into the master
development branch? Just curious, since they forked when you did that
last tag (this never happens!), and I sent another series of mtd-utils
fixes...I assume I should still base off of master, right?

Brian

Re: [BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Artem Bityutskiy]

On Fri, 2011-08-19 at 10:22 -0700, Brian Norris wrote:
> On Fri, Aug 19, 2011 at 10:02 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > On Thu, 2011-08-11 at 16:30 +0100, Fryar, Jeff wrote:
> >> mtd-utils nandwrite.c: After a page write failure, the calculation
> >> of the block number to erase is incorrect.  The erase block size is
> >> being passed as the erase block number in the call to mtd_erase().
> >
> > Looks like correct fix, thanks! Your patch is not very well formatted,
> > but I've amended it and applied to mtd-utils, thanks!
> 
> Oops, my bad on the bug. I didn't ever get around to using this
> feature when I updated nandwrite a while back. Thanks for catching it.
> 
> BTW, are you going to rebase or merge the v1.4.6 tag into the master
> development branch? Just curious, since they forked when you did that
> last tag (this never happens!), and I sent another series of mtd-utils
> fixes...I assume I should still base off of master, right?

Hi Brian,

as always, sorry for long delays, I have very little time for hobby
projects. I guess I'll end up disappearing from mtd at some point, but
will see, the plan is to still help MTD moving forward.

Sure you have to base on the master branch. The tag was indeed a "fork"
- I took tag v1.4.5 and put your patch on top. So v1.4.6 is not part of
the master branch. I did so because I did not want to include whole
bunch of changes which could add new regressions - I wanted to make just
a "bugfix" release.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [BUG] mtd-utils: nandwrite: invalid erase after page write failure

[Brian Norris]

On Fri, Aug 19, 2011 at 11:13 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> Hi Brian,
>
> as always, sorry for long delays, I have very little time for hobby
> projects. I guess I'll end up disappearing from mtd at some point, but
> will see, the plan is to still help MTD moving forward.

Well, this last e-mail was no delay at all! But I understand the delay
when it happens.

What will happen if you disappear? :(

> Sure you have to base on the master branch. The tag was indeed a "fork"
> - I took tag v1.4.5 and put your patch on top. So v1.4.6 is not part of
> the master branch. I did so because I did not want to include whole
> bunch of changes which could add new regressions - I wanted to make just
> a "bugfix" release.

Right, makes sense. I should have figured.

Brian