Re: [PATCH] btrfs-progs: tidy up cmd_subvol_create() whitespace & returns

[Wang Shilong <wangsl.fnst@cn.fujitsu.com>]

On 09/04/2013 09:24 AM, Eric Sandeen wrote:
> MOn Sep 3, 2013, at 8:22 PM, Wang Shilong <wangsl.fnst@cn.fujitsu.com> wrote:
>
>> On 09/03/2013 11:50 PM, Eric Sandeen wrote:
>>> On 9/3/13 8:13 AM, Wang Shilong wrote:
>>>> Hello Eric,
>>>>
>>>> Recently, i notice btrfs-progs's magic return value. For example, EACCESS return 12.
>>>> Magic return value is confusing for code reviewers. However,at least we should guarantee
>>>> all these conditions return the same value(for example 12 for this case)
>>>>
>>>> Or we can change all the magic number to 1. Miao reminded me that
>>>> there may be some applications that have catched and checked these
>>>> magic return values…
>>>>
>>>> Any ideas about this?
>>> I think that if we want to do anything different from the standard, expected
>>> "return 1 and set errno" then it needs to be a careful design decision, documented,
>>> used consistently, and tested.
>> I'd prefer to use standard approach.There are still many magic return
>> values unfixed.If
>> there are no objections, i would do like to correct the magic return
>> value following the rule:
>>
>> 0 No errors
>> 1 Usage or syntax error
>>
>> Exceptions come from btrfs scrub/fsck,getting such operations' status
>> etc.It is meaningful to
>> differ these return values.
>>
> Fsck in particular has some semi-standard, documented return values I think
See it, thanks for reminding^_^

Thanks,
Wang
>
> Eric
>
>> Thanks,
>> Wang
>>> As far as I can tell, what is in btrfs-progs today is undocumented,
>>> untested, and only occasionally/randomly used.  Therefore I don't
>>> think it's useful as it stands today, and why I had started removing
>>> them.
>>>
>>> -Eric
>>>
>>>> Thanks,
>>>> Wang
>>>>> From:
>>>>> Just whitespace fixes, and magical return value removal.
>>>>>
>>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>>> ---
>>>>> cmds-subvolume.c | 18 +++++++++---------
>>>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
>>>>> index 01b982c..a9999ac 100644
>>>>> --- a/cmds-subvolume.c
>>>>> +++ b/cmds-subvolume.c
>>>>> @@ -104,9 +104,9 @@ static int cmd_subvol_create(int argc, char **argv)
>>>>>     dst = argv[optind];
>>>>>
>>>>>     res = test_isdir(dst);
>>>>> -    if(res >= 0 ){
>>>>> +    if (res >= 0) {
>>>>>         fprintf(stderr, "ERROR: '%s' exists\n", dst);
>>>>> -        return 12;
>>>>> +        return 1;
>>>>>     }
>>>>>
>>>>>     newname = strdup(dst);
>>>>> @@ -114,24 +114,24 @@ static int cmd_subvol_create(int argc, char **argv)
>>>>>     dstdir = strdup(dst);
>>>>>     dstdir = dirname(dstdir);
>>>>>
>>>>> -    if( !strcmp(newname,".") || !strcmp(newname,"..") ||
>>>>> +    if (!strcmp(newname, ".") || !strcmp(newname, "..") ||
>>>>>          strchr(newname, '/') ){
>>>>>         fprintf(stderr, "ERROR: uncorrect subvolume name ('%s')\n",
>>>>>             newname);
>>>>> -        return 14;
>>>>> +        return 1;
>>>>>     }
>>>>>
>>>>>     len = strlen(newname);
>>>>>     if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
>>>>>         fprintf(stderr, "ERROR: subvolume name too long ('%s)\n",
>>>>>             newname);
>>>>> -        return 14;
>>>>> +        return 1;
>>>>>     }
>>>>>
>>>>>     fddst = open_file_or_dir(dstdir);
>>>>>     if (fddst < 0) {
>>>>>         fprintf(stderr, "ERROR: can't access to '%s'\n", dstdir);
>>>>> -        return 12;
>>>>> +        return 1;
>>>>>     }
>>>>>
>>>>>     printf("Create subvolume '%s/%s'\n", dstdir, newname);
>>>>> @@ -159,10 +159,10 @@ static int cmd_subvol_create(int argc, char **argv)
>>>>>     close(fddst);
>>>>>     free(inherit);
>>>>>
>>>>> -    if(res < 0 ){
>>>>> -        fprintf( stderr, "ERROR: cannot create subvolume - %s\n",
>>>>> +    if (res < 0) {
>>>>> +        fprintf(stderr, "ERROR: cannot create subvolume - %s\n",
>>>>>             strerror(e));
>>>>> -        return 11;
>>>>> +        return 1;
>>>>>     }
>>>>>
>>>>>     return 0;
>>>>> -- 
>>>>> 1.7.11.7
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html