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?

Thanks,
Hitoshi

-
        if (plug)
                sd_init_req(&hdr, SD_OP_MD_PLUG);
        else
--
1.8.3.2


--
sheepdog mailing list
sheepdog@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/sheepdog


--
sheepdog mailing list
sheepdog@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/sheepdog

Reply via email to