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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 12, 2019, 17:18 UTC
Message-ID: <xmqqh8d9diy8.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAPig+cTMoYMmnHVuay_4RGr_FN-HWC1tN4TPDsNM4vKCH3GzXw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 8 quoted lines

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

Show 6 quoted lines

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