From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cn.fujitsu.com ([222.73.24.84]:26884 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1761603Ab3IDBWk convert rfc822-to-8bit (ORCPT ); Tue, 3 Sep 2013 21:22:40 -0400 Message-ID: <52268AE3.4020107@cn.fujitsu.com> Date: Wed, 04 Sep 2013 09:20:35 +0800 From: Wang Shilong MIME-Version: 1.0 To: Eric Sandeen CC: Wang Shilong , "linux-btrfs@vger.kernel.org Btrfs" , "dsterba@suse.cz Sterba" , Miao Xie Subject: Re: [PATCH] btrfs-progs: tidy up cmd_subvol_create() whitespace & returns References: <1378213291-1034-1-git-send-email-wangshilong1991@gmail.com> <0A5B4F8C-8B08-4976-A660-6396407E932D@gmail.com> <52260537.6090103@redhat.com> In-Reply-To: <52260537.6090103@redhat.com> Content-Type: text/plain; charset=GB2312 Sender: linux-btrfs-owner@vger.kernel.org List-ID: 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. 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 >>> --- >>> 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 >>> >