At Thu, 07 Aug 2014 14:26:28 +0800,
Ruoyu wrote:
>
>
> On 2014年08月07日 13:39, Hitoshi Mitake wrote:
> > At Thu, 7 Aug 2014 12:08:23 +0800,
> > Ruoyu wrote:
> >> The purpose of the checking in the function is to ensure the
> >> argument is not null. But it is not needed any longer because
> >> the argument should be guaranteed in the framework as long as
> >> CMD_NEED_ARG is specified in subcommand.
> >>
> >> Since the problem is solved in commit e6d1706, we can remove
> >> the checking safely.
> >>
> >> Signed-off-by: Ruoyu <lian...@ucweb.com>
> >> ---
> >> dog/node.c | 8 --------
> >> 1 file changed, 8 deletions(-)
> >>
> >> diff --git a/dog/node.c b/dog/node.c
> >> index b6fd929..f9f7351 100644
> >> --- a/dog/node.c
> >> +++ b/dog/node.c
> >> @@ -441,14 +441,6 @@ static int do_plug_unplug(char *disks, bool plug)
> >> struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
> >> int ret;
> >>
> >> - if (!disks)
> >> - return EXIT_USAGE;
> >> -
> > I agree with that this check isn't required.
> >
> >> - if (!strlen(disks)) {
> >> - sd_err("Empty path isn't allowed");
> >> - return EXIT_FAILURE;
> >> - }
> > But I think this check is needed. If we execute a command like this:
> > dog node md plug "", sheep dies because sheep daemon assumes a path
> > name passed by dog process is correct.
> >
> > If you agree with this opinion, I can apply this patch with
> > eliminating the deletion of second check. How do you think?
> I think the above checking should be processed in framework, too.
> Otherwise, we must check it in every function that need arguments.
>
> I suggest in do_generic_subcommand:
>
> optind++;
> - if (flags & CMD_NEED_ARG && argc == optind)
> + if (flags & CMD_NEED_ARG && (argc == optind || argv[optind][0] == '\0'))
> subcommand_usage(argv[1], argv[2], EXIT_USAGE);
> ret = sub[i].fn(argc, argv);
>
> How do you think?
The idea looks good to me. If we add commands which allow empty string
as it argument in the future, let's prepare CMD_ALLOW_EMPTY_ARG like
the CMD_NEED_ARG flag. But currently your idea would work well.
Thanks,
Hitoshi
--
sheepdog mailing list
sheepdog@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/sheepdog
