| Message ID | 1420037925-31156-3-git-send-email-jbb5044@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 2014/12/31 22:58, Joe Balough wrote: > If flashing fails, ubiformat will exit with the value of 1. > If confirmation fails, ubiformat will exit with the value of 2. > > Signed-off-by: Joe Balough <jbb5044@gmail.com> > --- > ubi-utils/ubiformat.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c > index 4129404..3b8a22e 100644 > --- a/ubi-utils/ubiformat.c > +++ b/ubi-utils/ubiformat.c > @@ -548,7 +548,7 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd, > err = mtd_read(mtd, args.node_fd, eb, 0, read_buf, new_len); > if (err) { > sys_errmsg("cannot readback eraseblock %d for confirmation", eb); > - goto out_close; > + goto out_confirm_fail; > } > > if (memcmp(buf, read_buf, new_len) != 0) { > @@ -568,6 +568,10 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd, > out_close: > close(fd); > return -1; > + > +out_confirm_fail: > + close(fd); > + return -2; > } > > static int format(libmtd_t libmtd, const struct mtd_dev_info *mtd, > @@ -702,7 +706,7 @@ out_free: > > int main(int argc, char * const argv[]) > { > - int err, verbose; > + int err, verbose, confirm_fail = 0; > libmtd_t libmtd; > struct mtd_info mtd_info; > struct mtd_dev_info mtd; > @@ -932,6 +936,8 @@ int main(int argc, char * const argv[]) > > if (args.image) { > err = flash_image(libmtd, &mtd, &ui, si); > + if (err == -2) > + confirm_fail = 1; > if (err < 0) > goto out_free; > > @@ -955,5 +961,8 @@ out_close: > close(args.node_fd); > out_close_mtd: > libmtd_close(libmtd); > - return -1; > + if (confirm_fail) > + return 2; > + else > + return 1; > } > There is not need to return different values for any error cases, and a positive return value is not proper for a aborted case. Just print a error message at failure happening is enough, I think.
On 2015/1/5 23:07, Joseph Balough wrote: > > On Sat, Jan 3, 2015 at 9:38 PM, hujianyang <hujianyang@huawei.com <mailto:hujianyang@huawei.com>> wrote: > > > There is not need to return different values for any error cases, and a > positive return value is not proper for a aborted case. > > Just print a error message at failure happening is enough, I think. > > > It was useful for me to know what the specific error was -- A messed up ubi file could indicate a bad download while a confirm error could indicate bad flash. It's no problem for me, I can just leave my patch as something I apply locally. I do wonder about the current exit value on error. ubiformat currently exits -1 when there is an error. The exit status for programs in UNIX are 8-bit unsigned integers, so the program actually exits 255. Perhaps this should at least return 1 instead of -1. The exit status for programs in UNIX are 8-bit unsigned integers, so the program actually exits 255. Perhaps this should at least return 1 instead of -1. Yes, you are right~! Sorry for it.
diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c index 4129404..3b8a22e 100644 --- a/ubi-utils/ubiformat.c +++ b/ubi-utils/ubiformat.c @@ -548,7 +548,7 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd, err = mtd_read(mtd, args.node_fd, eb, 0, read_buf, new_len); if (err) { sys_errmsg("cannot readback eraseblock %d for confirmation", eb); - goto out_close; + goto out_confirm_fail; } if (memcmp(buf, read_buf, new_len) != 0) { @@ -568,6 +568,10 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd, out_close: close(fd); return -1; + +out_confirm_fail: + close(fd); + return -2; } static int format(libmtd_t libmtd, const struct mtd_dev_info *mtd, @@ -702,7 +706,7 @@ out_free: int main(int argc, char * const argv[]) { - int err, verbose; + int err, verbose, confirm_fail = 0; libmtd_t libmtd; struct mtd_info mtd_info; struct mtd_dev_info mtd; @@ -932,6 +936,8 @@ int main(int argc, char * const argv[]) if (args.image) { err = flash_image(libmtd, &mtd, &ui, si); + if (err == -2) + confirm_fail = 1; if (err < 0) goto out_free; @@ -955,5 +961,8 @@ out_close: close(args.node_fd); out_close_mtd: libmtd_close(libmtd); - return -1; + if (confirm_fail) + return 2; + else + return 1; }
If flashing fails, ubiformat will exit with the value of 1. If confirmation fails, ubiformat will exit with the value of 2. Signed-off-by: Joe Balough <jbb5044@gmail.com> --- ubi-utils/ubiformat.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)