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.
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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
