From owner-svn-src-head@FreeBSD.ORG Tue Feb 24 06:49:50 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B99FA106564A; Tue, 24 Feb 2009 06:49:50 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au [211.29.132.183]) by mx1.freebsd.org (Postfix) with ESMTP id 52B738FC16; Tue, 24 Feb 2009 06:49:50 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-107-120-227.carlnfd1.nsw.optusnet.com.au (c122-107-120-227.carlnfd1.nsw.optusnet.com.au [122.107.120.227]) by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id n1O6nlUQ030782 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 24 Feb 2009 17:49:48 +1100 Date: Tue, 24 Feb 2009 17:49:47 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Pawel Jakub Dawidek In-Reply-To: <200902230733.n1N7XTUB017956@svn.freebsd.org> Message-ID: <20090224172129.W10644@delplex.bde.org> References: <200902230733.n1N7XTUB017956@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r188934 - head/tools/regression/fstest X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Feb 2009 06:49:51 -0000 On Mon, 23 Feb 2009, Pawel Jakub Dawidek wrote: > Log: > Add explicit casting in few places. Most of these casts are excessive. > It is only really necessary for open(2)'s third argument, which is optional and > obtained through stdarg(3). This arg is variadic so a cast is needed. For all others, a prototype should be in scope so the casts have no effect except possibly to cause different warnings. > open(2)'s third argument is 32bit and we pass 64 > bits. On little endian it works, because we take lower 32 bits, but on big Actually, open()'s 3rd arg is the unary promotion of mode_t. This just happens to be 32 bits on all supported machines. > Modified: head/tools/regression/fstest/fstest.c > ============================================================================== > --- head/tools/regression/fstest/fstest.c Mon Feb 23 06:47:10 2009 (r188933) > +++ head/tools/regression/fstest/fstest.c Mon Feb 23 07:33:29 2009 (r188934) > @@ -337,7 +337,7 @@ show_stat(struct stat64 *sp, const char > printf("%lld", (long long)sp->st_ctime); > #ifdef HAS_CHFLAGS > else if (strcmp(what, "flags") == 0) > - printf("%s", flags2str(chflags_flags, sp->st_flags)); > + printf("%s", flags2str(chflags_flags, (long long)sp->st_flags)); > #endif Here the cast does nothing except possibly break warnings that flags2str() has a bogus type. st_flags has type fflags_t which happens to be unsigned 32 bits, while the function to print it takes a larger type with different signedness. Since the arg type happens to be larger on all supported machines, there are no problems with sign extension bugs. > else if (strcmp(what, "type") == 0) { > switch (sp->st_mode & S_IFMT) { > @@ -443,17 +443,17 @@ call_syscall(struct syscall_desc *scall, > fprintf(stderr, "too few arguments\n"); > exit(1); > } > - rval = open(STR(0), flags, (mode_t)NUM(2)); > + rval = open(STR(0), (int)flags, (mode_t)NUM(2)); > } else { > if (i == 3) { > fprintf(stderr, "too many arguments\n"); > exit(1); > } > - rval = open(STR(0), flags); > + rval = open(STR(0), (int)flags); > } > break; > case ACTION_CREATE: > - rval = open(STR(0), O_CREAT | O_EXCL, NUM(1)); > + rval = open(STR(0), O_CREAT | O_EXCL, (mode_t)NUM(1)); > if (rval >= 0) > close(rval); > break; THe casts for open() are needed, and you used the correct one (not to int32_t like the log message says). > @@ -461,7 +461,7 @@ call_syscall(struct syscall_desc *scall, > rval = unlink(STR(0)); > break; > case ACTION_MKDIR: > - rval = mkdir(STR(0), NUM(1)); > + rval = mkdir(STR(0), (mode_t)NUM(1)); > break; > case ACTION_RMDIR: > rval = rmdir(STR(0)); > @@ -476,30 +476,30 @@ call_syscall(struct syscall_desc *scall, > rval = rename(STR(0), STR(1)); > break; > case ACTION_MKFIFO: > - rval = mkfifo(STR(0), NUM(1)); > + rval = mkfifo(STR(0), (mode_t)NUM(1)); > break; > case ACTION_CHMOD: > - rval = chmod(STR(0), NUM(1)); > + rval = chmod(STR(0), (mode_t)NUM(1)); > break; > #ifdef HAS_LCHMOD > case ACTION_LCHMOD: > - rval = lchmod(STR(0), NUM(1)); > + rval = lchmod(STR(0), (mode_t)NUM(1)); > break; All the above functions should have a prototype in scope, so no casts are needed for the mode_t args. Casts would be needed to support K&R, but I bet this code isn't closed to working with a K&R compiler since the necessary casts are still missing in most places. Please add them in all places or none :-). > #endif > case ACTION_CHOWN: > - rval = chown(STR(0), NUM(1), NUM(2)); > + rval = chown(STR(0), (uid_t)NUM(1), (gid_t)NUM(2)); > break; > case ACTION_LCHOWN: > - rval = lchown(STR(0), NUM(1), NUM(2)); > + rval = lchown(STR(0), (uid_t)NUM(1), (gid_t)NUM(2)); > break; Here the casts have no effect since a prototype is in scope. > #ifdef HAS_CHFLAGS > case ACTION_CHFLAGS: > - rval = chflags(STR(0), str2flags(chflags_flags, STR(1))); > + rval = chflags(STR(0), (unsigned long)str2flags(chflags_flags, STR(1))); This excessive cast also causes a line longer than 80 characters. > break; > #endif > #ifdef HAS_LCHFLAGS > case ACTION_LCHFLAGS: > - rval = lchflags(STR(0), str2flags(chflags_flags, STR(1))); > + rval = lchflags(STR(0), (unsigned long)str2flags(chflags_flags, STR(1))); > break; > #endif > case ACTION_TRUNCATE: > This excessive cast also causes a line longer than 80 characters, and doesn't even match the prototype (lchflags() still takes an int arg for historical reasons). syscalls.master still misdeclares both chflags() and lchflags() and some other syscalls as taking an int. For lchflags(), this is bug for bug compatible with the prototype, so it works in practice even in the big endian case, but for chflags() it would break in the same way as for open() in the big endian case if longs were 64 bits. FreeBSD supports some machines with 64 bit longs but all of these happen to be little endian so chflags() works for all supported machines. Note that chflags() couldn't be fixed by casting in the call -- the prototype would still extend to 64 bits if longs are 64 bits. Bruce