[sheepdog] [PATCH] dog/node: remove checking argument in md plug/unplug since root cause is found
Ruoyu
liangry at ucweb.com
Thu Aug 7 08:26:28 CEST 2014
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 <liangry at 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 at lists.wpkg.org
>> http://lists.wpkg.org/mailman/listinfo/sheepdog
More information about the sheepdog
mailing list