[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCHv2] nandwrite: new mtd-utils applet
From: Denys Vlasenko <vda.linux () googlemail ! com>
Date: 2010-08-24 11:31:51
Message-ID: AANLkTimyKDZLfO_TjTWm9r0E1Kf1W9F2-Qx9+yzpTGRo () mail ! gmail ! com
[Download RAW message or body]
On Tue, Aug 24, 2010 at 7:35 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> + char *mtd_device, *img = NULL;
(*) see below.
> + opt_complementary = "-1";
Hmm... why not go further and not use "-1:?2" then...
> + opts = getopt32(argv, "ps:", &opt_s);
> + if (opts & OPTION_S)
> + mtdoffset = bb_strtou(opt_s, NULL, 0);
> + argv += optind;
> + if (argv[1] && argv[2])
> + bb_show_usage();
... in order to drop the above test.
> + mtd_device = argv[0];
> + if (argv[1])
> + img = argv[1];
And here, just do img = argv[1]; without checking for NULL :)
Then, remove NULL assignment above (*).
> + if (cnt == 0)
> + break; /* we are done */
Does this also apply to -p? What if I write a 100M image to
200M mtd - should it stop, or should it write zeros till the end?
(Even if the behavior is correct, please add a comment and
explain this detail there).
> + if (cnt < meminfo.writesize && !(opts & OPTION_P))
> + bb_error_msg_and_die("Unexpected EOF, "
> + "use padding");
> + else if (cnt < meminfo.writesize)
> + memset(filebuf + cnt, 0, meminfo.writesize - cnt);
(0) perhaps this way:
if (cnt < meminfo.writesize) {
if (!(opts & OPTION_P)))
bb_error_msg_and_die(...);
memset(filebuf + cnt, 0, meminfo.writesize - cnt);
}
(1) what the message tries to say? "short read, use -p to zero pad" perhaps?
(2) msg should start in lowercase (all other places in bbox do it that way).
--
vda
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic