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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 3, 2025, 17:05 UTC
Message-ID: <CAPig+cT1dQL+MfUctyw=9O5Wd2yUqA40pXSgsRHKfNf=6vxQ7w@mail.gmail.com>
In-Reply-To: <20250403144852.19153-1-sn03.general@gmail.com>

On Thu, Apr 3, 2025 at 10:52 AM Subhaditya Nath <sn03.general@gmail.com> wrote:

Show 7 quoted lines

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

Show 12 quoted lines

> Signed-off-by: Subhaditya Nath <sn03.general@gmail.com>
> ---
> 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`.