From owner-svn-src-all@FreeBSD.ORG Sun Mar 24 12:56:03 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id D3A05FC8; Sun, 24 Mar 2013 12:56:03 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx07.syd.optusnet.com.au (fallbackmx07.syd.optusnet.com.au [211.29.132.9]) by mx1.freebsd.org (Postfix) with ESMTP id 700BA9D3; Sun, 24 Mar 2013 12:56:02 +0000 (UTC) Received: from mail36.syd.optusnet.com.au (mail36.syd.optusnet.com.au [211.29.133.76]) by fallbackmx07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r2OCtrHP020144; Sun, 24 Mar 2013 23:55:53 +1100 Received: from c211-30-173-106.carlnfd1.nsw.optusnet.com.au (c211-30-173-106.carlnfd1.nsw.optusnet.com.au [211.30.173.106]) by mail36.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id r2OCtikn017904 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 24 Mar 2013 23:55:45 +1100 Date: Sun, 24 Mar 2013 23:55:44 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Sean Bruno Subject: Re: svn commit: r248680 - head/sbin/fsck_ffs In-Reply-To: <201303241041.r2OAfTr1033109@svn.freebsd.org> Message-ID: <20130324232715.L959@besplex.bde.org> References: <201303241041.r2OAfTr1033109@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=D5QfsYtj c=1 sm=1 a=jO6rb6iV1_cA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=0pm9H42DGT4A:10 a=wn9u6872aJiteysuqiwA:9 a=CjuIK1q_8ugA:10 a=TEtd8y5WR3g2ypngnwZWYw==:117 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 24 Mar 2013 12:56:03 -0000 On Sun, 24 Mar 2013, Sean Bruno wrote: > Log: > Resolve clang compile errors on amd64/i386 for certain by casting. > > compile tested with clang on i386, amd64 > compile tested with gcc on i386, amd64, sparc64 > > Submitted by: delphij > > Modified: > head/sbin/fsck_ffs/fsutil.c > > Modified: head/sbin/fsck_ffs/fsutil.c > ============================================================================== > --- head/sbin/fsck_ffs/fsutil.c Sun Mar 24 10:14:25 2013 (r248679) > +++ head/sbin/fsck_ffs/fsutil.c Sun Mar 24 10:41:29 2013 (r248680) > @@ -507,8 +507,8 @@ static void printIOstats(void) > > clock_gettime(CLOCK_REALTIME_PRECISE, &finishpass); > timespecsub(&finishpass, &startpass); > - printf("Running time: %ld.%03ld msec\n", > - finishpass.tv_sec, finishpass.tv_nsec / 1000000); > + printf("Running time: %jd.%03ld msec\n", > + (intmax_t)finishpass.tv_sec, finishpass.tv_nsec / 1000000); > printf("buffer reads by type:\n"); > for (totalmsec = 0, i = 0; i < BT_NUMBUFTYPES; i++) > totalmsec += readtime[i].tv_sec * 1000 + Casting to intmax_t is excessive. It gives at least 64 bits, but 16 bits suffice for runtimes of up to 9.1 hours. 32 bits suffice for tuntimes of up to a measly 68 years. The following bugs remain in the changed statement: - the description claims that the runtime is in msec. It is actually in seconds with a precision of milliseconds. - non-KNF indentation in continued line. > @@ -519,10 +519,10 @@ static void printIOstats(void) > if (readcnt[i] == 0) > continue; > msec = readtime[i].tv_sec * 1000 + readtime[i].tv_nsec / 1000000; Line too long. > - printf("%21s:%8ld %2ld.%ld%% %4ld.%03ld sec %2lld.%lld%%\n", > + printf("%21s:%8ld %2ld.%ld%% %4jd.%03ld sec %2lld.%lld%%\n", > buftype[i], readcnt[i], readcnt[i] * 100 / diskreads, > (readcnt[i] * 1000 / diskreads) % 10, > - readtime[i].tv_sec, readtime[i].tv_nsec / 1000000, > + (intmax_t)readtime[i].tv_sec, readtime[i].tv_nsec / 1000000, > msec * 100 / totalmsec, (msec * 1000 / totalmsec) % 10); Simimlarly for the new cast. Now the units are correct. Now the indentation is normal. Now the long long abomination is used for the some of the times. This matches the long long abomination being used for the type of msec and totalmsec. The use of long long is inconsistent with the use of intmax_t, and almost as excessive. Now 16 bit ints are too small (they only work for 32 seconds), and 32-bit ints are only enough for 24 days, but that is more than enough. Except when calculating the average, the multiplication by 1000 would overflow after 35 minutes with 32-bit ints. Division by 0 seems to occur for runtimes of < 1 msec. Since the clock id is CLOCK_REALTIME_*, this can occur even for long actual runtimes, if the clock is stepped backwards. I don't like the poor man's floating point calculations. Everything is easier using floating point. > } > printf("\n"); > Bruce