Re: [Patch v1 2/3] t5318: replace use of /dev/zero with generate_zero_bytes

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sat, Feb 9, 2019 at 2:00 PM <randall.s.becker@rogers.com> wrote:
>> This change removes the dependency on /dev/zero with generate_zero_bytes
>> appending NUL values to blocks generating wrong signatures for test cases.
>
> This commit message says what the patch does but not _why_. At
> minimum, it should explain that /dev/zero is not available on all
> platforms, therefore, not portable, and (perhaps) cite NonStop as an
> example.

Does sombody want to do the honors?  [PATCH 1/3] would become wasted
effort until that happens.  On the other hand, if this is not urgent
(it is only urgent for those without /dev/zero, and to others it may
be distraction/disruption this close to the final release to add
increased risk of fat finger mistakes), obviously I can wait.

>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
>> ---
>> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
>> @@ -383,7 +383,7 @@ corrupt_graph_and_verify() {
>> -       dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=$(($orig_size - $zero_pos)) &&
>> +       generate_zero_bytes $(($orig_size - $zero_pos)) >> "$objdir/info/commit-graph" &&

The original says "skip to the $zero_pos location in an existing
info/commit-graph file, and overwrite its existing contents up to
the $orig_size location with NULs".

If I recall the implementation of generate_zero_bytes in [PATCH 1/3]
correctly, it just generated stream of NULs of given number of bytes.

The only reason why this rewrite is not wrong is because the
previous line (omitted in your quote) truncates the file to
$zero_pos.  That is a bit tricky ;-).