Subject: Re: [PATCH] t7422: remove extraneous argument to printf

From: Junio C Hamano <[email-masked]>
Date: Wed, 09 Apr 2025 09:19:19 -0700

Eric Sunshine <[email-masked]> writes:

> On Thu, Apr 3, 2025 at 10:52 AM Subhaditya Nath <[email-masked]> wrote:
>> The POSIX man page of printf(1) mentions -
>> > If the format operand contains no conversion specifications and
>> > argument operands are present, the results are unspecified.
>>
>> In practice, this means some printf implementations throw an error
>> when provided with extra operands, thereby causing the test to fail
>> erroneously. This commit fixes that issue.
>
> Thanks, this makes sense.
>
>> Signed-off-by: Subhaditya Nath <[email-masked]>
>> ---
>> diff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh
>> @@ -180,7 +180,7 @@ test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE'
>>                 COMMIT=$(git rev-parse HEAD) &&
>>                 for i in $(test_seq 2000)
>>                 do
>> -                       printf "[submodule \"sm-$i\"]\npath = recursive-submodule-path-$i\n" "$i" ||
>> +                       printf "[submodule \"sm-$i\"]\npath = recursive-submodule-path-$i\n" ||
>>                         return 1
>>                 done >gitmodules &&
>>                 BLOB=$(git hash-object -w --stdin <gitmodules) &&
>
> This change is obviously correct.
>
> This was added by 65f586132b (t7422: fix flaky test caused by buffered
> stdout, 2025-01-10) which also added a similar loop just below this
> one:
>
>     for i in $(test_seq 2000)
>     do
>         printf "160000 commit $COMMIT\trecursive-submodule-path-%d\n" "$i" ||
>         return 1
>     done >>tree &&
>
> in which the loop variable is interpolated indirectly via `%d` rather
> than directly via `$i`. I suspect that the author's intention was to
> use `%d` for both loops. Thus, for the sake of consistency and to
> match the author's original intent, it may make more sense to retain
> the argument to printf and instead employ `%d`.

Yup, I think, even though both would be correct, such a change would
be more sensible than the one that was posted here.

Thanks.