Subject: Re: bug#9157: [PATCH] dd: sparse conv flag

From: Pádraig Brady <[email-masked]>
Date: Wed, 29 Feb 2012 00:36:15 +0000

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.