On 02/28/2012 11:39 PM, Paul Eggert wrote: > On 02/28/2012 03:13 PM, Jim Meyering wrote: >>> -#define OUTPUT_BLOCK_SLOP (page_size - 1) >>>> +#define OUTPUT_BLOCK_SLOP MAX (sizeof (uintptr_t), page_size - 1) >> I haven't seen justification for why you're making the above change. >> Can sizeof uintptr_t really be larger than page_size-1 (getpagesize()-1)? >> This seems so unlikely that it'd deserve an assertion in main where >> page_size is set, even though there are only two uses of OUTPUT_BLOCK_SLOP. > > Theoretically getpagesize could return 1, no? > Admittedly it's unlikely. I guess it'd be OK > to omit this change, and to just put in a comment > saying something like "We use the output slop > to store a uintptr_t value, so we assume that > getpagesize returns a value greater than sizeof (uintptr_t)." > Perhaps the comment should go right next to the place where > the uintptr_t sentinel is inserted. > >> If you leave the sizeof, please omit the parentheses. > > But "sizeof uintptr_t" wouldn't work, as uintptr_t is a type. >
Ok I'm going with the version with the assert, since that's better than a comment. thanks for all the comments, Pádraig. p.s. I can't commit at the moment due to authentication error. I'll look into it in the morning.
