| Message ID | 1468833493-28311-1-git-send-email-zajec5@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Jul 18, 2016 at 11:18 AM, Rafał Miłecki <zajec5@gmail.com> wrote: > Errors may happen, it's e.g. easy on embedded devices to run out of space > when dumping big partitions. This patch adds a helper function for > writing. It deals with partial writes and just returns 0 on success or > error number. > > The old code didn't check for errors at all which could result in > incomplete dumps without exiting with an error. Is this patch different from my version, does it fix more? http://lists.infradead.org/pipermail/linux-mtd/2016-April/067234.html Brian, if this is fine with you I'd apply one of these patches.
On 18 July 2016 at 13:33, Richard Weinberger <richard.weinberger@gmail.com> wrote: > On Mon, Jul 18, 2016 at 11:18 AM, Rafał Miłecki <zajec5@gmail.com> wrote: >> Errors may happen, it's e.g. easy on embedded devices to run out of space >> when dumping big partitions. This patch adds a helper function for >> writing. It deals with partial writes and just returns 0 on success or >> error number. >> >> The old code didn't check for errors at all which could result in >> incomplete dumps without exiting with an error. > > Is this patch different from my version, does it fix more? > http://lists.infradead.org/pipermail/linux-mtd/2016-April/067234.html > > Brian, if this is fine with you I'd apply one of these patches. I wasn't aware of your patch, having it accepted would save me some time for sure. Two minor advantages I see in my version: 1) It handles partial writes, just retries if only some part of buffer has been written 2) It displays error number and string which may provide some extra hint to the user In my case (when I was running out of space), the successful last write was partial because there wasn't enough space for the whole buffer. I needed to retry a write to get a real error code.
Rafał, Am 18.07.2016 um 13:59 schrieb Rafał Miłecki: > On 18 July 2016 at 13:33, Richard Weinberger > <richard.weinberger@gmail.com> wrote: >> On Mon, Jul 18, 2016 at 11:18 AM, Rafał Miłecki <zajec5@gmail.com> wrote: >>> Errors may happen, it's e.g. easy on embedded devices to run out of space >>> when dumping big partitions. This patch adds a helper function for >>> writing. It deals with partial writes and just returns 0 on success or >>> error number. >>> >>> The old code didn't check for errors at all which could result in >>> incomplete dumps without exiting with an error. >> >> Is this patch different from my version, does it fix more? >> http://lists.infradead.org/pipermail/linux-mtd/2016-April/067234.html >> >> Brian, if this is fine with you I'd apply one of these patches. > > I wasn't aware of your patch, having it accepted would save me some > time for sure. > > Two minor advantages I see in my version: > 1) It handles partial writes, just retries if only some part of buffer > has been written That's a plus point. :-) > 2) It displays error number and string which may provide some extra > hint to the user My patch too since it uses mtd-util's logging functions. sys_errmsg(...) Could you please merge both patches and resend? Thanks, //richard
On 18 July 2016 at 16:10, Richard Weinberger <richard@nod.at> wrote: > Rafał, > > Am 18.07.2016 um 13:59 schrieb Rafał Miłecki: >> On 18 July 2016 at 13:33, Richard Weinberger >> <richard.weinberger@gmail.com> wrote: >>> On Mon, Jul 18, 2016 at 11:18 AM, Rafał Miłecki <zajec5@gmail.com> wrote: >>>> Errors may happen, it's e.g. easy on embedded devices to run out of space >>>> when dumping big partitions. This patch adds a helper function for >>>> writing. It deals with partial writes and just returns 0 on success or >>>> error number. >>>> >>>> The old code didn't check for errors at all which could result in >>>> incomplete dumps without exiting with an error. >>> >>> Is this patch different from my version, does it fix more? >>> http://lists.infradead.org/pipermail/linux-mtd/2016-April/067234.html >>> >>> Brian, if this is fine with you I'd apply one of these patches. >> >> I wasn't aware of your patch, having it accepted would save me some >> time for sure. >> >> Two minor advantages I see in my version: >> 1) It handles partial writes, just retries if only some part of buffer >> has been written > > That's a plus point. :-) > >> 2) It displays error number and string which may provide some extra >> hint to the user > > My patch too since it uses mtd-util's logging functions. > sys_errmsg(...) > > Could you please merge both patches and resend? Oh, I didn't realize sys_errmsg does that. I assumed it logs into system log (which seemed like a weird idea to do). A bit confusing name. Thanks, I'll send V2.
Rafał, Am 18.07.2016 um 16:51 schrieb Rafał Miłecki: > On 18 July 2016 at 16:10, Richard Weinberger <richard@nod.at> wrote: >> My patch too since it uses mtd-util's logging functions. >> sys_errmsg(...) >> >> Could you please merge both patches and resend? > > Oh, I didn't realize sys_errmsg does that. I assumed it logs into > system log (which seemed like a weird idea to do). A bit confusing > name. Thanks, I'll send V2. > The sys_ prefix denotes that this function is used for syscall errors. :-) Thanks, //richard
diff --git a/nand-utils/nanddump.c b/nand-utils/nanddump.c index 4ee7ed4..0bbc97f 100644 --- a/nand-utils/nanddump.c +++ b/nand-utils/nanddump.c @@ -293,6 +293,25 @@ nil: linebuf[lx++] = '\0'; } +/** + * ofd_write - writes whole buffer to the file associated with a descriptor + * + * On failure an error (negative number) is returned. Otherwise 0 is returned. + */ +static int ofd_write(int ofd, const unsigned char *buf, size_t nbyte) +{ + ssize_t bytes; + + while (nbyte) { + bytes = write(ofd, buf, nbyte); + if (bytes < 0) + return -errno; + buf += bytes; + nbyte -= bytes; + } + + return 0; +} /* * Main program @@ -309,6 +328,7 @@ int main(int argc, char * const argv[]) bool eccstats = false; unsigned char *readbuf = NULL, *oobbuf = NULL; libmtd_t mtd_desc; + int err; process_options(argc, argv); @@ -443,10 +463,19 @@ int main(int argc, char * const argv[]) for (i = 0; i < bs; i += PRETTY_ROW_SIZE) { pretty_dump_to_buffer(readbuf + i, PRETTY_ROW_SIZE, pretty_buf, PRETTY_BUF_LEN, true, canonical, ofs + i); - write(ofd, pretty_buf, strlen(pretty_buf)); + err = write(ofd, pretty_buf, strlen(pretty_buf)); + if (err) { + errmsg("ofd_write: %s\n", strerror(-err)); + goto closeall; + } } - } else - write(ofd, readbuf, bs); + } else { + err = ofd_write(ofd, readbuf, bs); + if (err) { + errmsg("ofd_write: %d %s\n", err, strerror(-err)); + goto closeall; + } + } if (omitoob) continue; @@ -466,10 +495,19 @@ int main(int argc, char * const argv[]) for (i = 0; i < mtd.oob_size; i += PRETTY_ROW_SIZE) { pretty_dump_to_buffer(oobbuf + i, mtd.oob_size - i, pretty_buf, PRETTY_BUF_LEN, false, canonical, 0); - write(ofd, pretty_buf, strlen(pretty_buf)); + err = write(ofd, pretty_buf, strlen(pretty_buf)); + if (err) { + errmsg("ofd_write: %s\n", strerror(-err)); + goto closeall; + } } - } else - write(ofd, oobbuf, mtd.oob_size); + } else { + err = ofd_write(ofd, oobbuf, mtd.oob_size); + if (err) { + errmsg("ofd_write: %d %s\n", err, strerror(-err)); + goto closeall; + } + } } /* Close the output file and MTD device, free memory */
Errors may happen, it's e.g. easy on embedded devices to run out of space when dumping big partitions. This patch adds a helper function for writing. It deals with partial writes and just returns 0 on success or error number. The old code didn't check for errors at all which could result in incomplete dumps without exiting with an error. Signed-off-by: Rafał Miłecki <zajec5@gmail.com> --- nand-utils/nanddump.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 6 deletions(-)