[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Pádraig Brady
Subject: bug#9157: [PATCH] dd: sparse conv flag
Date: Tue, 28 Feb 2012 23:33:40 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]