[PATCHv2] nandwrite: new mtd-utils applet

Denys Vlasenko vda.linux at googlemail.com
Tue Aug 24 11:31:51 UTC 2010


On Tue, Aug 24, 2010 at 7:35 AM, Baruch Siach <baruch at 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


More information about the busybox mailing list