From: Todd Zullinger <tmz@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
Justin Tobler <jltobler@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH 2/2] test-lib: teach test_seq the -f option
Date: Tue, 24 Jun 2025 09:42:52 -0400 [thread overview]
Message-ID: <aFqrXMHStEJXx8e5@teonanacatl.net> (raw)
In-Reply-To: <20250624101651.GC636332@coredump.intra.peff.net>
Jeff King wrote:
> On Mon, Jun 23, 2025 at 01:27:07PM -0400, Todd Zullinger wrote:
>> I don't know whether it's worth the extra code or not. I
>> just wondered about how it would fail in the face of a minor
>> typo. It certainly should cause any test to fail if it were
>> to output 1 instead of the intended format string, so it's
>> arguably fine as-is.
>
> Hmm, maybe. I notice that "seq" itself does this (though it did surprise
> me). I think there it is actually doing the "%" interpolation itself (to
> avoid memory errors by feeding arbitrary strings to printf functions),
> so it's easy to do.
>
> In our case, we can rely on the shell printf to do something sensible if
> fed garbage. And because we're not parsing ourselves, a pattern like you
> have above isn't totally accurate (e.g., consider what it would with
> "%%d"). But it probably would be enough to catch typos.
Parsing it fully felt like overkill, and a good bit more
code to do it well. So that was me being lazy. :)
> It would also disallow:
>
> test_seq -f "same line" 50
>
> to produce repeated lines, though I don't know how valuable that would
> be. So I dunno.
I can see someone using it that way, like a cheap version of
the yes command.
> If people are going to use "-f%d", I think we'd be better off making it
> work than trying to complain about it. But I was hoping we could just
> keep things simple and stupid, given the limited audience.
>
> So my inclination is to leave the sharp edges and see if anybody gets
> cut, but it's possible that I'm just being lazy.
Sounds good to me. As you say, it's not going to be exposed
to "normal" users.
The worst thing it can do is allow a test to pass which
shouldn't -- and that's by far the least likely way for the
(not very) sharp edges to cut us.
--
Todd
next prev parent reply other threads:[~2025-06-24 13:42 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-23 10:55 [PATCH 0/2] test_seq format option Jeff King
2025-06-23 10:55 ` [PATCH 1/2] t7422: replace confusing printf with echo Jeff King
2025-06-23 17:59 ` Eric Sunshine
2025-06-24 10:05 ` Jeff King
2025-06-23 10:56 ` [PATCH 2/2] test-lib: teach test_seq the -f option Jeff King
2025-06-23 16:10 ` Junio C Hamano
2025-06-23 16:25 ` Justin Tobler
2025-06-24 10:11 ` [PATCH 3/2] test-lib: document test_seq's "-f" option Jeff King
2025-06-25 0:02 ` Justin Tobler
2025-06-23 17:27 ` [PATCH 2/2] test-lib: teach test_seq the -f option Todd Zullinger
2025-06-24 10:16 ` Jeff King
2025-06-24 13:42 ` Todd Zullinger [this message]
2025-06-24 6:22 ` Eric Sunshine
2025-06-24 10:36 ` Jeff King
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=aFqrXMHStEJXx8e5@teonanacatl.net \
--to=tmz@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jltobler@gmail.com \
--cc=peff@peff.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox