| Message ID | h0mppp$7v2$1@ger.gmane.org |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2009-06-09 at 16:04 -0700, Jehan Bing wrote: > - readlen = meminfo.writesize; > > - if (ifd != STDIN_FILENO) { > - int tinycnt = 0; > - > - if (pad && (imglen < readlen)) > - { > - readlen = imglen; > - erase_buffer(writebuf + readlen, meminfo.writesize - readlen); > - } > + { > + readlen = meminfo.writesize; > Err, why do you need these spare { } ? > - /* Read Page Data from input file */ > - while(tinycnt < readlen) { > - cnt = read(ifd, writebuf + tinycnt, readlen - tinycnt); > - if (cnt == 0) { // EOF > - break; > - } else if (cnt < 0) { > - perror ("File I/O error on input file"); > - goto closeall; > - } > - tinycnt += cnt; > - } > - } else { > int tinycnt = 0; Err, is it normal C do do stuff like { readlen = meminfo.writesize; int tinycnt += cnt; ? I think this is C++.
On Wed, 2009-06-10 at 19:03 +0300, Artem Bityutskiy wrote: > Err, is it normal C do do stuff like > { > readlen = meminfo.writesize; > int tinycnt += cnt; Sorry, I meant { readlen = meminfo.writesize; int tinycnt = 0;
Artem Bityutskiy wrote: > On Tue, 2009-06-09 at 16:04 -0700, Jehan Bing wrote: > >> - readlen = meminfo.writesize; >> >> - if (ifd != STDIN_FILENO) { >> - int tinycnt = 0; >> - >> - if (pad && (imglen < readlen)) >> - { >> - readlen = imglen; >> - erase_buffer(writebuf + readlen, meminfo.writesize - readlen); >> - } >> + { >> + readlen = meminfo.writesize; >> >> > > Err, why do you need these spare { } ? > Right, I wanted to comment on that but forgot by the time I started the email. My idea was to make the patches clearer. Patch 3/3 puts that code inside a condition. So to avoid extra changes just because of the indentation, I added the extra { } in this patch instead since I was already heavily modifying it anyway. Do you prefer me to remove them? >> - /* Read Page Data from input file */ >> - while(tinycnt < readlen) { >> - cnt = read(ifd, writebuf + tinycnt, readlen - tinycnt); >> - if (cnt == 0) { // EOF >> - break; >> - } else if (cnt < 0) { >> - perror ("File I/O error on input file"); >> - goto closeall; >> - } >> - tinycnt += cnt; >> - } >> - } else { >> int tinycnt = 0; >> > > Err, is it normal C do do stuff like > { > readlen = meminfo.writesize; > int tinycnt += cnt; > > ? I think this is C++. > gcc didn't complain. Easy to fix.
Artem Bityutskiy wrote: > On Wed, 2009-06-10 at 19:03 +0300, Artem Bityutskiy wrote: > > Err, is it normal C do do stuff like > > { > > readlen = meminfo.writesize; > > int tinycnt += cnt; > > Sorry, I meant > { > readlen = meminfo.writesize; > int tinycnt = 0; It's C99, and GCC has accepted that syntax since GCC 3.0. If you want to disable it, use -Werror=declaration-after-statement in current GCC, or -pedantic (but that has other effects). -- Jamie
On Tue, 2009-06-09 at 16:04 -0700, Jehan Bing wrote: > Use same code path for reading data (not the OOB) from either the > standard input or a regular file. > > Signed-off-by: Jehan Bing <jehan@orb.com> The patches look OK to me, but I do not have time to review them very really well. So would it please be possible to describe how you tested them to convince me they are ok? Then I'd push them to mtd-utils.git tree. Did you test writing with/without oob, from file/stdin, etc? Thanks!
--- a/nandwrite.c 2009-06-08 13:33:32.000000000 -0700 +++ b/nandwrite.c 2009-06-09 13:15:17.000000000 -0700 @@ -260,7 +260,6 @@ int main(int argc, char * const argv[]) int ret, readlen; int oobinfochanged = 0; struct nand_oobinfo old_oobinfo; - int readcnt = 0; bool failed = true; process_options(argc, argv); @@ -476,37 +475,18 @@ int main(int argc, char * const argv[]) } - readlen = meminfo.writesize; - if (ifd != STDIN_FILENO) { - int tinycnt = 0; - - if (pad && (imglen < readlen)) - { - readlen = imglen; - erase_buffer(writebuf + readlen, meminfo.writesize - readlen); - } + { + readlen = meminfo.writesize; - /* Read Page Data from input file */ - while(tinycnt < readlen) { - cnt = read(ifd, writebuf + tinycnt, readlen - tinycnt); - if (cnt == 0) { // EOF - break; - } else if (cnt < 0) { - perror ("File I/O error on input file"); - goto closeall; - } - tinycnt += cnt; - } - } else { int tinycnt = 0; - while(tinycnt < readlen) { + while (tinycnt < readlen) { cnt = read(ifd, writebuf + tinycnt, readlen - tinycnt); if (cnt == 0) { // EOF break; } else if (cnt < 0) { - perror ("File I/O error on stdin"); + perror ("File I/O error on input"); goto closeall; } tinycnt += cnt; @@ -514,18 +494,30 @@ int main(int argc, char * const argv[]) /* No padding needed - we are done */ if (tinycnt == 0) { - imglen = 0; + // For standard input, set the imglen to 0 to signal + // the end of the "file". For non standard input, leave + // it as-is to detect an early EOF + if (ifd == STDIN_FILENO) { + imglen = 0; + } break; } - /* No more bytes - we are done after writing the remaining bytes */ - if (cnt == 0) { - imglen = 0; - } /* Padding */ - if (pad && (tinycnt < readlen)) { - erase_buffer(writebuf + tinycnt, meminfo.writesize - tinycnt); + if (tinycnt < readlen) { + if (!pad) { + fprintf(stderr, "Unexpected EOF. Expecting at least " + "%d more bytes. Use the padding option.\n", + readlen - tinycnt); + goto closeall; + } + erase_buffer(writebuf + tinycnt, readlen - tinycnt); + } + + /* No more bytes - we are done after writing the remaining bytes */ + if ((ifd == STDIN_FILENO) && (cnt == 0)) { + imglen = 0; } }
Use same code path for reading data (not the OOB) from either the standard input or a regular file. Signed-off-by: Jehan Bing <jehan@orb.com>