Subject: off_t madness
To: None <tech-misc@NetBSD.org>
From: Christian Biere <christianbiere@gmx.de>
List: tech-misc
Date: 02/05/2007 19:17:47
Hi,

the following shows what can wrong if off_t is used incorrectly in comparisons.
I'm not sure whether it really yields a warning on 64-bit archs (like AMD64),
it does for me if I fake a 64-bit size_t on a 32-bit arch. Regardless of this
warning, things can still go wrong as shown below.

May I assume option 2 is the preferred way to write code like this?

Technically, I think the compiler warning is wrong because off_t is implicitely
promoted to a non-existing unsigned integer type (uint64_t in our case) or if
size_t was wider, it would be converted to size_t. Thus, this looks like a
false-positive to me. Careless use of explicit casts here can cause real bugs
though (i.e., off_t should never be cast to size_t unless you've already
checked that it fits).

#include <inttypes.h>
#include <stdlib.h>
#include <stdio.h>
#include <limits.h>

#define MAX_INT_VAL_STEP(t) \
	((t) 1 << (CHAR_BIT * sizeof(t) - 1 - ((t) -1 < 1)))
	 
#define MAX_INT_VAL(t) \
	((MAX_INT_VAL_STEP(t) - 1) + MAX_INT_VAL_STEP(t))

#define MIN_INT_VAL(t) \
	((t) -MAX_INT_VAL(t) - 1)

/* uint64_t for 64-bit arch, uint32_t for 32-bit arch */
typedef uint32_t my_size_t;

#define MY_SIZE_T_MAX MAX_INT_VAL(my_size_t)
#define OFF_T_MAX MAX_INT_VAL(off_t)

int main(void)
{
	my_size_t bufsize;
	off_t filesize;
	int ret;

	bufsize = -1;
	filesize = OFF_T_MAX;

	/*
	 * Question: 
	 *
	 * Does the file fit into the buffer? (example: mmap())
	 *
	 * ret = filesize >= 0 && filesize <= bufsize;
	 *
	 * Problem:
	 *
	 * - comparison between signed and unsigned.
	 *
	 * - off_t is a signed integer type, an equivalent unsigned integer
	 *   type (unsigned off_t?) does not exist.
	 *
	 * - size_t is architecture-dependent.
	 *
	 * - off_t is currently always int64_t on BSDs but not everywhere
	 *   (int32_t on many legacy systems) and may change (int128_t?).
	 *
	 * - OFF_T_MAX (maximum value of off_t) is unknown but can be
	 *   inferred making a few assumptions that should be true in
	 *   practice.
	 */

	/* Option 1 */

	ret = OFF_T_MAX > MY_SIZE_T_MAX
		? (filesize >= 0 && filesize <= (off_t)bufsize)
		: (filesize >= 0 && (my_size_t)filesize <= bufsize);

	/*
	 * WRONG if OFF_T_MAX is wrong.
	 */

	printf("1) filesize >= 0 && filesize <= bufsize: %s\n",
		ret ? "yes" : "no");

	/* Option 2 */

	ret = filesize >= 0
		&& (uintmax_t)(intmax_t)filesize <= (uintmax_t)bufsize;

	/*
	 * WRONG if off_t is an extended integer type not covered by
	 * intmax_t. This is probably rather unlikely, so it might be
	 * the preferrable solution for ISO C99 code.
	 *
	 * example:
	 *
	 * typedef int64_t intmax_t;
	 * typedef __int128_t off_t;
	 */

	printf("2) filesize >= 0 && filesize <= bufsize: %s\n",
		ret ? "yes" : "no");

	/* Option 3 */

	ret = filesize > 0 && filesize <= (off_t)bufsize;

	/*
	 * WRONG if OFF_T_MAX < SIZE_MAX
	 *
	 * example:
	 *
	 * typedef int64_t off_t;
	 * typedef uint64_t size_t;
	 *
	 * or:
	 * typedef int32_t off_t;
	 * typedef uint64_t size_t;
	 *
	 */

	printf("3) filesize >= 0 && filesize <= bufsize: %s\n",
		ret ? "yes" : "no");

	/* Option 4 */

	ret = filesize >= 0 && (size_t)filesize <= bufsize;

	/*
	 * WRONG if SIZE_MAX < OFF_T_MAX
	 *
	 * example:
	 *
	 * typedef int64_t off_t;
	 * typedef uint64_t size_t;
	 *
	 * or:
	 * typedef int64_t off_t;
	 * typedef uint32_t size_t;
	 */

	printf("4) filesize >= 0 && filesize <= bufsize: %s\n",
		ret ? "yes" : "no");

	/* Option 5 */

	ret = filesize >= 0 && filesize <= bufsize;

	/*
	 * Just ignore the compiler warning.
	 */

	printf("5) filesize >= 0 && filesize <= bufsize: %s\n",
		ret ? "yes" : "no");

	return 0;
}