Subject: Re: off_t madness
To: None <tech-misc@NetBSD.org>
From: Christian Biere <christianbiere@gmx.de>
List: tech-misc
Date: 02/05/2007 21:31:32
Alan Barrett wrote:
> On Mon, 05 Feb 2007, Christian Biere wrote:
> > #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)
> 
> Without comments, it's too difficult to figure out what you are trying
> to do here.

/*
 * These macros determine the maximum/minimum value of the given integer type
 * "t". This works for signed as well as unsigned types. This code does
 * carefully avoid integer overflows and undefined behaviour.
 * However, it's assumed the type consists of exactly sizeof(type) * CHAR_BIT
 * bits.
 */
 
> > 	/* Option 2 */
> > 
> > 	ret = filesize >= 0
> > 		&& (uintmax_t)(intmax_t)filesize <= (uintmax_t)bufsize;
 
> This looks right (thought I would use parentheses around the && term).

The (uintmax_t) before bufsize is actually unnecessary and so it would
fit nicely into one line.

I rather avoid unnecessary parentheses (except in macros) because I find
too many of them less readable and it can also suppress valid compiler
warnings. Consider examples like this:

	ret = (filesize =! 0);
	ret = (filesize = 0);

I would also normally use a inline function to avoid explicit casts in
random places:

static inline uintmax_t
off_to_uintmax(off_t offset)
{
	return (uintmax_t)(intmax_t)offset;
}

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

> intmax_t and unintmax_t are defined as the longest integral types (section
> 1.18.1.5 of draft N1124 of the C99 standard).  There may be extended types
> longet than "long long" or longer than int64_t, but not longer then intmax_t.

Ok, thanks for your comments.

-- 
Christian