Date: Sun, 24 Mar 2013 23:55:44 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Sean Bruno <sbruno@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r248680 - head/sbin/fsck_ffs Message-ID: <20130324232715.L959@besplex.bde.org> In-Reply-To: <201303241041.r2OAfTr1033109@svn.freebsd.org> References: <201303241041.r2OAfTr1033109@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130324232715.L959>