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

[Pádraig Brady]

On 02/28/2012 11:13 PM, Jim Meyering wrote:
> Pádraig Brady wrote:
> ...
> 
> Thanks for working on that.
> All I would adjust are a few nits and
> add require_sparse_support_ in the test script:
> 
>> Subject: [PATCH] dd: add support for the conv=sparse option
>>
>> Small seeks are not coalesced to larger ones (like is
>> done in cache_round() for example, for the moment at least.
>>
>> conv= is used rather then oflag= for FreeBSD compatibility.
>>
>> * src/dd.c (last_seek): A new global boolean to flag
> 
> "last" is ambiguous.  Does it mean "final" or "preceding"?
>>From the context below, (not the comment, since it too uses "last")
> I see it means "final".  "final_op_was_seek" is longer but ultra clear.
> Maybe worth the length.

yes I was too terse

>> -#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.

Yes it's never going to trigger.
Paul suggested MAX(...) so as to doc/enforce the new constraint.
assert() is not used in dd.c yet
I'll leave as is unless Paul comments otherwise (modulo the
extraneous () of course).

cheers,
Pádraig