svn commit: r248680 - head/sbin/fsck_ffs

[Bruce Evans]

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