Suggested patch: reset errno after isatty()

Suggested patch: reset errno after isatty()

[Ketil Froyn]

isatty() uses an ioctl and the resulting error code to determine if an
fd is a tty or not. If it isn't, errno is set to ENOTTY. Later in the
code, pread() fails, or rather returns 0 immediately. When this
happens, the following perror("pread") tells me:

pread: Not a typewriter

which is the wrong error. Here's strace showing the issue:

open("/sdcard/mtd5.dump", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0644) = 4
ioctl(4, TCGETS or SNDCTL_TMR_TIMEBASE, 0xbee43ab0) = -1 ENOTTY (Not a
typewriter)
write(2, "Block size 131072, page size 204"..., 47) = 47
write(2, "Dumping data starting at 0x00000"..., 64) = 64
ioctl(3, 0x40084d0b, 0xbee43c30)        = 0
pread(3, "", 2048, 0)                   = 0
write(2, "pread", 5)                    = 5
write(2, ": ", 2)                       = 2
write(2, "Not a typewriter", 16)        = 16
write(2, "\n", 1)                       = 1

And the included patch (below, against v1.4.1, but simple enough)
should take care of it.

Though if someone knows what could be causing the pread() to keep
failing, I'd be interested in hearing it! And the mtd device is
reported as having a page size of 2048 and oob size 56, so to get even
this far I added that as a valid page size. Isn't it?

diff --git a/nanddump.c b/nanddump.c
index 709b2db..ba370ad 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -374,6 +374,7 @@ int main(int argc, char * const argv[])
                close(fd);
                exit(EXIT_FAILURE);
        }
+       errno = 0;

        /* Initialize start/end addresses and block size */
        if (length)

Re: Suggested patch: reset errno after isatty()

[Matthieu CASTET]

Ketil Froyn a écrit :
> isatty() uses an ioctl and the resulting error code to determine if an
> fd is a tty or not. If it isn't, errno is set to ENOTTY. Later in the
> code, pread() fails, or rather returns 0 immediately. When this
> happens, the following perror("pread") tells me:
> 
> pread: Not a typewriter
> 
> which is the wrong error. Here's strace showing the issue:
> 
> open("/sdcard/mtd5.dump", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0644) = 4
> ioctl(4, TCGETS or SNDCTL_TMR_TIMEBASE, 0xbee43ab0) = -1 ENOTTY (Not a
> typewriter)
> write(2, "Block size 131072, page size 204"..., 47) = 47
> write(2, "Dumping data starting at 0x00000"..., 64) = 64
> ioctl(3, 0x40084d0b, 0xbee43c30)        = 0
> pread(3, "", 2048, 0)                   = 0
> write(2, "pread", 5)                    = 5
> write(2, ": ", 2)                       = 2
> write(2, "Not a typewriter", 16)        = 16
> write(2, "\n", 1)                       = 1
> 
> And the included patch (below, against v1.4.1, but simple enough)
> should take care of it.
> 
> Though if someone knows what could be causing the pread() to keep
> failing, I'd be interested in hearing it! And the mtd device is
> reported as having a page size of 2048 and oob size 56, so to get even
> this far I added that as a valid page size. Isn't it?
Why errno is checked when pread return 0 ?
Errno should be checked only when return is -1.

man pread :
RETURN VALUE
        On success, the number of bytes read or written is returned 
(zero indi-
        cates  that  nothing  was  written,  in the case of pwrite(), or 
end of
        file, in the case of pread()), or -1 on error, in which case 
errno  is
        set to indicate the error.

Re: Suggested patch: reset errno after isatty()

[Ketil Froyn]

On Wed, Nov 3, 2010 at 9:23 AM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:

> Why errno is checked when pread return 0 ?
> Errno should be checked only when return is -1.

Good point, and I guess that was why I was getting weird errors in the
first place, because errno was checked when no error had occurred. Odd
that pread() returned 0 when trying to read on my android device -
maybe a bionic bug?

This code has been rewritten, but the new master isn't working for me
(yet). This patch against v1.4.1 seems to have solved my problems for
now. I've just replaced pread() with read(), because it works, and
fixed up the error checking.

--- a/nanddump.c
+++ b/nanddump.c
@@ -412,10 +412,25 @@ int main(int argc, char * const argv[])
                        memset (readbuf, 0xff, bs);
                } else {
                        /* Read page data and exit on failure */
-                       if (pread(fd, readbuf, bs, ofs) != bs) {
-                               perror("pread");
-                               goto closeall;
-                       }
+                       do {
+                               ret = read(fd, readbuf, bs);
+                               if (ret == -1) {
+                                       if (errno == EAGAIN || errno == EINTR) {
+                                               continue;
+                                       }
+                                       perror("read");
+                                       goto closeall;
+                               }
+                               if (ret == 0) {
+                                       printf("No more data to read\n");
+                                       break;
+                               }
+                               if (ret != bs) {
+                                       fprintf(stderr, "read() got
wrong number of bytes: got %i, expected %i\n", ret, bs);
+                                       goto closeall;
+                               }
+                               break;
+                       } while (1);
                }

                /* ECC stats available ? */

Re: Suggested patch: reset errno after isatty()

[Mike Frysinger]

On Wed, Nov 3, 2010 at 09:16, Ketil Froyn wrote:
> On Wed, Nov 3, 2010 at 9:23 AM, Matthieu CASTET wrote:
> This code has been rewritten, but the new master isn't working for me
> (yet). This patch against v1.4.1 seems to have solved my problems for
> now. I've just replaced pread() with read(), because it works, and
> fixed up the error checking.
>
> --- a/nanddump.c
> +++ b/nanddump.c
> @@ -412,10 +412,25 @@ int main(int argc, char * const argv[])
>                        memset (readbuf, 0xff, bs);
>                } else {
>                        /* Read page data and exit on failure */
> -                       if (pread(fd, readbuf, bs, ofs) != bs) {
> -                               perror("pread");
> -                               goto closeall;
> -                       }
> +                       do {
> +                               ret = read(fd, readbuf, bs);
> +                               if (ret == -1) {
> +                                       if (errno == EAGAIN || errno == EINTR) {
> +                                               continue;
> +                                       }
> +                                       perror("read");
> +                                       goto closeall;
> +                               }
> +                               if (ret == 0) {
> +                                       printf("No more data to read\n");
> +                                       break;
> +                               }
> +                               if (ret != bs) {
> +                                       fprintf(stderr, "read() got
> wrong number of bytes: got %i, expected %i\n", ret, bs);
> +                                       goto closeall;
> +                               }
> +                               break;
> +                       } while (1);
>                }

doesnt libmtd provide read functions already ?
-mike

Re: Suggested patch: reset errno after isatty()

[Ketil Froyn]

On Sat, Nov 6, 2010 at 9:54 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
>
> doesnt libmtd provide read functions already ?
> -mike

I'm not familiar with libmtd, but the latest release (v1.4.1) just did
it itself with pread, and since that failed I fixed it.

The new code in the repo, which appears to use libmtd, didn't work for
me, and the fix appeared more complicated. My patch is for v1.4.1,
which should also make it easier to integrate for package maintainers,
if they wish, pending a working rewrite.

Ketil

Re: Suggested patch: reset errno after isatty()

[Mike Frysinger]

On Sun, Nov 7, 2010 at 17:36, Ketil Froyn wrote:
> On Sat, Nov 6, 2010 at 9:54 AM, Mike Frysinger wrote:
>> doesnt libmtd provide read functions already ?
>
> I'm not familiar with libmtd, but the latest release (v1.4.1) just did
> it itself with pread, and since that failed I fixed it.
>
> The new code in the repo, which appears to use libmtd, didn't work for
> me, and the fix appeared more complicated. My patch is for v1.4.1,
> which should also make it easier to integrate for package maintainers,
> if they wish, pending a working rewrite.

the focus is on the master branch.  stable fixes should largely be
back ports, not a complete fork.
-mike

Re: Suggested patch: reset errno after isatty()

[Artem Bityutskiy]

On Sun, 2010-11-07 at 23:36 +0100, Ketil Froyn wrote:
> On Sat, Nov 6, 2010 at 9:54 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> >
> > doesnt libmtd provide read functions already ?
> > -mike
> 
> I'm not familiar with libmtd, but the latest release (v1.4.1) just did
> it itself with pread, and since that failed I fixed it.
> 
> The new code in the repo, which appears to use libmtd, didn't work for
> me, and the fix appeared more complicated. My patch is for v1.4.1,
> which should also make it easier to integrate for package maintainers,
> if they wish, pending a working rewrite.

Ketil, you have a point, 

however, Mike is right, fixes should be back-ports, or if you fix a
problem in the older release, you should fix it in the master branch as
well, or show that it does not exist in the master branch. This is the
only way it is possible to keep development under control.

We really need to be sure that whatever is fixed in older releases is
also fixed in the newer. Yes, this is more work for you but hey, I'm
also doing this stuff in my spare time.

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

Re: Suggested patch: reset errno after isatty()

[Artem Bityutskiy]

On Sat, 2010-11-13 at 13:07 +0200, Artem Bityutskiy wrote:
> On Sun, 2010-11-07 at 23:36 +0100, Ketil Froyn wrote:
> > On Sat, Nov 6, 2010 at 9:54 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> > >
> > > doesnt libmtd provide read functions already ?
> > > -mike
> > 
> > I'm not familiar with libmtd, but the latest release (v1.4.1) just did
> > it itself with pread, and since that failed I fixed it.
> > 
> > The new code in the repo, which appears to use libmtd, didn't work for
> > me, and the fix appeared more complicated. My patch is for v1.4.1,
> > which should also make it easier to integrate for package maintainers,
> > if they wish, pending a working rewrite.
> 
> Ketil, you have a point, 
> 
> however, Mike is right, fixes should be back-ports, or if you fix a
> problem in the older release, you should fix it in the master branch as
> well, or show that it does not exist in the master branch. This is the
> only way it is possible to keep development under control.
> 
> We really need to be sure that whatever is fixed in older releases is
> also fixed in the newer. Yes, this is more work for you but hey, I'm
> also doing this stuff in my spare time.

BTW, with patches from Brian Norris which I just pushed the master
branch may work better for you.

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

Re: Suggested patch: reset errno after isatty()

[Ketil Froyn]

On Sat, Nov 13, 2010 at 12:55 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Sat, 2010-11-13 at 13:07 +0200, Artem Bityutskiy wrote:
>>
>> however, Mike is right, fixes should be back-ports, or if you fix a
>> problem in the older release, you should fix it in the master branch as
>> well, or show that it does not exist in the master branch. This is the
>> only way it is possible to keep development under control.
>>
>> We really need to be sure that whatever is fixed in older releases is
>> also fixed in the newer. Yes, this is more work for you but hey, I'm
>> also doing this stuff in my spare time.
>
> BTW, with patches from Brian Norris which I just pushed the master
> branch may work better for you.

I've managed to build mtd-utils for my android device, using
codesourcery's toolchain. I built a static nanddump executable, copied
it to my device, and ran it. Here's what happened:

# nanddump -f /sdcard/mtd5.dump /dev/mtd/mtd5
ECC failed: 0
ECC corrected: 0
Number of bad blocks: 0
Number of bbt blocks: 0
Block size 131072, page size 2048, OOB size 0
Dumping data starting at 0x00000000 and ending at 0x0a5c0000...
Segmentation fault

The OOB size shown is 0, which is wrong, while the detected block size
and page size are correct. The old version used to complain:

# nanddump -f /sdcard/mtd5.dump /dev/mtd/mtd5
Unknown flash (not normal NAND)

until I did some debugging and found out that my device has an OOB
size of 56... This patch against v1.4.1 fixed that version:

--- a/nanddump.c
+++ b/nanddump.c
@@ -308,6 +308,7 @@ int main(int argc, char * const argv[])
                        !(meminfo.oobsize == 128 && meminfo.writesize == 4096) &
                        !(meminfo.oobsize == 64 && meminfo.writesize == 4096) &&
                        !(meminfo.oobsize == 64 && meminfo.writesize == 2048) &&
+                       !(meminfo.oobsize == 56 && meminfo.writesize == 2048) &&
                        !(meminfo.oobsize == 32 && meminfo.writesize == 1024) &&
                        !(meminfo.oobsize == 16 && meminfo.writesize == 512) &&
                        !(meminfo.oobsize == 8 && meminfo.writesize == 256)) {


I've done some digging around in the new code, but I haven't worked
out exactly why the OOB size is set to 0. I haven't figured out
exactly what it does yet... Or could it have anything to do with my
build environment?

Cheers, Ketil

Re: Suggested patch: reset errno after isatty()

[Ketil Froyn]

On Wed, Nov 17, 2010 at 4:50 PM, Ketil Froyn <ketil@froyn.name> wrote:
>
> I've done some digging around in the new code, but I haven't worked
> out exactly why the OOB size is set to 0. I haven't figured out
> exactly what it does yet... Or could it have anything to do with my
> build environment?

I rebuilt nanddump from v1.4.1 with my patches using the CodeSourcery
toolchain (I'd been using the AOSP toolchain before). It works fine,
so I guess that rules out problems in my build environment. Here's
what the run looks like:

/ # nanddump-old -f /sdcard/mtd5.dump /dev/mtd/mtd5
ECC failed: 0
ECC corrected: 0
Number of bad blocks: 0
Number of bbt blocks: 0
Block size 131072, page size 2048, OOB size 56
Dumping data starting at 0x00000000 and ending at 0x0a5c0000...
/ #

This particular system appears to use MTD partitioning, maybe
redboot(?) or something like that. I haven't had the opportunity to
delve into how that works yet - could it be that the partitioning
takes 8 bytes of the OOB?

Cheers, Ketil

Re: Suggested patch: reset errno after isatty()

[Mike Frysinger]

On Thu, Nov 18, 2010 at 06:13, Ketil Froyn wrote:
> On Wed, Nov 17, 2010 at 4:50 PM, Ketil Froyn wrote:
>> I've done some digging around in the new code, but I haven't worked
>> out exactly why the OOB size is set to 0. I haven't figured out
>> exactly what it does yet... Or could it have anything to do with my
>> build environment?
>
> I rebuilt nanddump from v1.4.1 with my patches using the CodeSourcery
> toolchain (I'd been using the AOSP toolchain before). It works fine,
> so I guess that rules out problems in my build environment. Here's
> what the run looks like:
>
> / # nanddump-old -f /sdcard/mtd5.dump /dev/mtd/mtd5
> ECC failed: 0
> ECC corrected: 0
> Number of bad blocks: 0
> Number of bbt blocks: 0
> Block size 131072, page size 2048, OOB size 56
> Dumping data starting at 0x00000000 and ending at 0x0a5c0000...
> / #

so you'll want to dive into the ioctls to see what isnt working right.
 strace is your friend.

> This particular system appears to use MTD partitioning, maybe
> redboot(?) or something like that. I haven't had the opportunity to
> delve into how that works yet - could it be that the partitioning
> takes 8 bytes of the OOB?

no
-mike

Re: Suggested patch: reset errno after isatty()

[Ketil Froyn]

On Wed, Nov 24, 2010 at 8:50 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Thu, Nov 18, 2010 at 06:13, Ketil Froyn wrote:
>
> so you'll want to dive into the ioctls to see what isnt working right.
>  strace is your friend.

Thanks for the kick. I had tried that, but for some reason strace was
dying with the error "syscall: unknown syscall trap 0xe8bd8008". A
little more searching now led me to this patch, which fixed it for me:

http://www.fluff.org/ben/patches/strace/strace-fix-arm-bad-syscall.patch

Anyway, here's the full output:

execve("/system/nanddump", ["/system/nanddump", "-f", "/sdcard/dump",
"/dev/mtd/mtd5"], [/* 6 vars */]) = 0
uname({sys="Linux", node="localhost", ...}) = 0
brk(0)                                  = 0x94000
brk(0x94d00)                            = 0x94d00
syscall_983045(0x944c0, 0x917d0, 0xffffffe0, 0x10, 0x92048, 0x8, 0x10,
0xf0005, 0x9185c, 0x4, 0x28, 0x944c0, 0, 0xbeccad00, 0xe474, 0xe484,
0x40000010, 0x944c0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) = 0
brk(0xb5d00)                            = 0xb5d00
brk(0xb6000)                            = 0xb6000
open("/sys/class/mtd", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|0x80000) = 3
fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
pivot_root(0x3, "")                     = 384
close(3)                                = 0
open("/sys/class/mtd/mtd0/name", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No
such file or directory)
open("/dev/mtd/mtd5", O_RDONLY|O_LARGEFILE) = 3
stat64("/dev/mtd/mtd5", {st_mode=S_IFCHR|0600, st_rdev=makedev(90,
10), ...}) = 0
open("/dev/mtd/mtd5", O_RDWR|O_LARGEFILE) = 4
ioctl(4, 0x80204d01, 0xbecca978)        = 0
ioctl(4, 0x40084d0b, 0xbecca998)        = 0
close(4)                                = 0
open("/proc/mtd", O_RDONLY|O_LARGEFILE) = 4
read(4, "dev:    size   erasesize  name\nmtd0: 00040000 00020000
\"misc\"\nmtd1: 00500000 00020000 \"recovery\"\nmtd2: 00280000
00020000 \"boot\"\nmtd3: 0aa00000 00020000 \"system\"\nmtd4: 08200000
00020000 \"cache\"\nmtd5: 0a5c0000 00020000 \"userdata\"\n", 4096) =
228
close(4)                                = 0
ioctl(3, 0x80104d12, 0xbeccacfc)        = 0
write(2, "ECC failed: 0\n", 14)         = 14
write(2, "ECC corrected: 0\n", 17)      = 17
write(2, "Number of bad blocks: 0\n", 24) = 24
write(2, "Number of bbt blocks: 0\n", 24) = 24
open("/sdcard/dump", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0644) = 4
ioctl(4, TCGETS or SNDCTL_TMR_TIMEBASE, 0xbecca9ec) = -1 ENOTTY (Not a
typewriter)
write(2, "Block size 131072, page size 2048, OOB size 0\n", 46) = 46
write(2, "Dumping data starting at 0x00000000 and ending at
0x0a5c0000...\n", 64) = 64
ioctl(3, 0x40084d0b, 0xbeccaa48)        = 0
_llseek(3, 0, [0], SEEK_SET)            = 0
read(3, "[snip]"..., 2048) = 2048
ioctl(3, 0x80104d12, 0xbeccacec)        = 0
write(4, "[snip]"..., 2048) = 2048
ioctl(3, 0xc0184d16, 0xbecca9e8)        = -1 ENOTTY (Not a typewriter)
ioctl(3, 0xc00c4d04 <unfinished ...>
+++ killed by SIGSEGV +++

The old version of nanddump (with my patches) works fine on the same
system. It looks like this:

execve("/system/nanddump-old", ["/system/nanddump-old", "-f",
"/sdcard/old-dump", "/dev/mtd/mtd5"], [/* 6 vars */]) = 0
uname({sys="Linux", node="localhost", ...}) = 0
brk(0)                                  = 0x96000
brk(0x96d00)                            = 0x96d00
syscall_983045(0x964c0, 0x92010, 0xffffffe0, 0x10, 0x939a0, 0x8, 0x10,
0xf0005, 0x920c8, 0x4, 0x28, 0x964c0, 0, 0xbeeb6cf0, 0xb0b4, 0xb0c4,
0x40000010, 0x964c0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) = 0
brk(0xb7d00)                            = 0xb7d00
brk(0xb8000)                            = 0xb8000
open("/dev/mtd/mtd5", O_RDONLY|O_LARGEFILE) = 3
ioctl(3, 0x80204d01, 0xbeeb6ca4)        = 0
ioctl(3, 0x80104d12, 0xbeeb6cd4)        = 0
write(2, "ECC failed: 0\n", 14)         = 14
write(2, "ECC corrected: 0\n", 17)      = 17
write(2, "Number of bad blocks: 0\n", 24) = 24
write(2, "Number of bbt blocks: 0\n", 24) = 24
open("/sdcard/old-dump", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0644) = 4
ioctl(4, TCGETS or SNDCTL_TMR_TIMEBASE, 0xbeeb6aec) = -1 ENOTTY (Not a
typewriter)
write(2, "Block size 131072, page size 2048, OOB size 56\n", 47) = 47
write(2, "Dumping data starting at 0x00000000 and ending at
0x0a5c0000...\n", 64) = 64
ioctl(3, 0x40084d0b, 0xbeeb6cf0)        = 0
read(3, "[snip]"..., 2048) = 2048
ioctl(3, 0x80104d12, 0xbeeb6cc4)        = 0
write(4, "[snip]"..., 2048) = 2048
ioctl(3, 0xc00c4d04, 0xbeeb6ce4)        = 0
write(4, "[snip]", 56) = 56
read(3, "[snip]"..., 2048) = 2048
ioctl(3, 0x80104d12, 0xbeeb6cc4)        = 0
write(4, "[snip]"..., 2048) = 2048
ioctl(3, 0xc00c4d04, 0xbeeb6ce4)        = 0
write(4, "[snip]", 56) = 56
etc...

>> This particular system appears to use MTD partitioning, maybe
>> redboot(?) or something like that. I haven't had the opportunity to
>> delve into how that works yet - could it be that the partitioning
>> takes 8 bytes of the OOB?
>
> no

Thanks for the clarification. I guess those are the actual physical
parameters, then.

Cheers, Ketil

Re: Suggested patch: reset errno after isatty()

[Artem Bityutskiy]

On Wed, 2010-11-24 at 10:59 +0100, Ketil Froyn wrote:
> On Wed, Nov 24, 2010 at 8:50 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> > On Thu, Nov 18, 2010 at 06:13, Ketil Froyn wrote:
> >
> > so you'll want to dive into the ioctls to see what isnt working right.
> >  strace is your friend.
> 
> Thanks for the kick. I had tried that, but for some reason strace was
> dying with the error "syscall: unknown syscall trap 0xe8bd8008". A
> little more searching now led me to this patch, which fixed it for me:
> 
> http://www.fluff.org/ben/patches/strace/strace-fix-arm-bad-syscall.patch
> 
> Anyway, here's the full output:

So it dies very soon. You should easily find the line of code where this
happens with gdb. Just compile nanddump with -g -O0

But I guess we are calling some new ioctl on the system which does not
support it, and do not fall-back to the new ioctl. We tried to be
careful about this, though.

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

Re: Suggested patch: reset errno after isatty()

[Ketil Froyn]

On Wed, Nov 24, 2010 at 3:12 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> So it dies very soon. You should easily find the line of code where this
> happens with gdb. Just compile nanddump with -g -O0

It's just that I have limited tools on my device where this happens.
But I've finally pinned it!

There's a couple of issues, I guess. Firstly, calling mtd_read_oob()
with an oob_size of 0 segfaults, which I guess it shouldn't. I haven't
looked into that. The problem here was that the oob_size was set to 0,
and that happens because the legacy code forgot to set it. This patch
fixes it for me:

diff --git a/lib/libmtd_legacy.c b/lib/libmtd_legacy.c
index 7488275..d6c3938 100644
--- a/lib/libmtd_legacy.c
+++ b/lib/libmtd_legacy.c
@@ -261,6 +261,7 @@ int legacy_get_dev_info(const char *node, struct
mtd_dev_info *mtd)
        mtd->size = ui.size;
        mtd->eb_size = ui.erasesize;
        mtd->min_io_size = ui.writesize;
+       mtd->oob_size = ui.oobsize;

        if (mtd->min_io_size <= 0) {
                errmsg("mtd%d (%s) has insane min. I/O unit size %d",


Cheers, Ketil

Re: Suggested patch: reset errno after isatty()

[Mike Frysinger]

On Wed, Nov 24, 2010 at 09:30, Ketil Froyn wrote:
> On Wed, Nov 24, 2010 at 3:12 PM, Artem Bityutskiy wrote:
>>
>> So it dies very soon. You should easily find the line of code where this
>> happens with gdb. Just compile nanddump with -g -O0
>
> It's just that I have limited tools on my device where this happens.
> But I've finally pinned it!
>
> There's a couple of issues, I guess. Firstly, calling mtd_read_oob()
> with an oob_size of 0 segfaults, which I guess it shouldn't. I haven't
> looked into that. The problem here was that the oob_size was set to 0,
> and that happens because the legacy code forgot to set it. This patch
> fixes it for me:

awesome.  can you post your signed-off-by tag and/or a proper git patch please ?
-mike

Re: Suggested patch: reset errno after isatty()

[Ketil Froyn]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

On Wed, Nov 24, 2010 at 10:36 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
>
> awesome.  can you post your signed-off-by tag and/or a proper git patch please ?
> -mike

I'm pretty new to git. Hope this is right.

Cheers, Ketil

[-- Attachment #2: 0001-mtd-utils-libmtd-legacy-code-OOB.patch --]
[-- Type: text/x-diff, Size: 827 bytes --]

From aea696a42d118598ca582180a89c7e5c0fe13b4e Mon Sep 17 00:00:00 2001
From: Ketil Froyn <ketil@froyn.name>
Date: Thu, 25 Nov 2010 09:52:12 +0100
Subject: [PATCH] mtd-utils: libmtd: legacy code OOB

legacy_get_dev_info() forgot to set the OOB size

Signed-off-by: Ketil Froyn <ketil@froyn.name>
---
 lib/libmtd_legacy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/libmtd_legacy.c b/lib/libmtd_legacy.c
index 7488275..d6c3938 100644
--- a/lib/libmtd_legacy.c
+++ b/lib/libmtd_legacy.c
@@ -261,6 +261,7 @@ int legacy_get_dev_info(const char *node, struct mtd_dev_info *mtd)
 	mtd->size = ui.size;
 	mtd->eb_size = ui.erasesize;
 	mtd->min_io_size = ui.writesize;
+	mtd->oob_size = ui.oobsize;
 
 	if (mtd->min_io_size <= 0) {
 		errmsg("mtd%d (%s) has insane min. I/O unit size %d",
-- 
1.7.0.4

Re: Suggested patch: reset errno after isatty()

[Artem Bityutskiy]

On Thu, 2010-11-25 at 09:58 +0100, Ketil Froyn wrote:
> On Wed, Nov 24, 2010 at 10:36 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> >
> > awesome.  can you post your signed-off-by tag and/or a proper git patch please ?
> > -mike
> 
> I'm pretty new to git. Hope this is right.

Pushed this patch to the mtd-utils tree, thanks!

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