From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Subhaditya Nath <sn03.general@gmail.com>,
git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH] t7422: remove extraneous argument to printf
Date: Wed, 09 Apr 2025 09:19:19 -0700 [thread overview]
Message-ID: <xmqqfrihs3y0.fsf@gitster.g> (raw)
In-Reply-To: <CAPig+cT1dQL+MfUctyw=9O5Wd2yUqA40pXSgsRHKfNf=6vxQ7w@mail.gmail.com> (Eric Sunshine's message of "Thu, 3 Apr 2025 13:05:16 -0400")
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Thu, Apr 3, 2025 at 10:52 AM Subhaditya Nath <sn03.general@gmail.com> 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 <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`.
Yup, I think, even though both would be correct, such a change would
be more sensible than the one that was posted here.
Thanks.
next prev parent reply other threads:[~2025-04-09 16:19 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-03 14:48 [PATCH] t7422: remove extraneous argument to printf Subhaditya Nath
2025-04-03 17:05 ` Eric Sunshine
2025-04-09 16:19 ` Junio C Hamano [this message]
2025-04-09 16:29 ` Subhaditya Nath
2025-04-09 16:32 ` Subhaditya Nath
2025-04-09 17:00 ` Eric Sunshine
2025-04-09 18:06 ` Junio C Hamano
2025-04-09 19:11 ` [PATCH] t7422: fix extra printf argument, eliminate loops Subhaditya Nath
2025-04-09 19:11 ` Subhaditya Nath
2025-04-10 12:39 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqfrihs3y0.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
--cc=sn03.general@gmail.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.