Hi Guillem,
On Mon, Oct 05, 2009 at 07:26:34PM +0200, Guillem Jover wrote:
> > +print_pretty(struct wipe_desc *wp, int line)
> > +{
> > + if (!line) {
> > + fprintf(stdout, "offset type\n");
> > + fprintf(stdout, "----------------------------------------------------------------\n");
> > + }
> > +
> > + fprintf(stdout, "0x%-17jx %s [%s]", wp->offset, wp->type, wp->usage);
> > +
> > + if (wp->label && *wp->label)
> > + fprintf(stdout, "\n%27s %s", "LABEL:", wp->label);
> > + if (wp->uuid)
> > + fprintf(stdout, "\n%27s %s", "UUID: ", wp->uuid);
> > + fputs("\n\n", stdout);
> > +}
>
> Why not just printf (and puts w/ one less '\n')?
Good catch. Fixed.
> > +static inline char *xstrdup(const char *s)
> [...]
> > +static void __attribute__((__noreturn__)) usage(FILE *out)
>
> Most of the others functions have the return type and specifiers on a
> different line.
OK.
> > +{
> > + fprintf(out, _("Usage: %s [options] <filename>\n\nOptions:\n"),
> > + program_invocation_short_name);
> > +
> > + fprintf(out, _(
> > + " -a, --all wipe all magic strings (BE CAREFUL!)\n"
> > + " -f, --fake everything to be done except for the write() call\n"
>
> Even if mount has the same option, it feels a bit confusing, most of
> the time -f tends to be used for --force, and usually --fake is
> spelled -n, --dry-run or --no-act (less so --simulate).
Good point, -n, --no-act sounds good.
> > +int
> > +main(int argc, char **argv)
> > +{
> > + struct wipe_desc *wp = NULL;
> > + int c, all = 0, has_offset = 0, fake = 0, mode = 0;
> > + char *fname;
>
> Make this const?
OK.
Fixed and merged into the master branch. Thanks for review.
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
