[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