From: Seonghyeon Cho <seonghyeoncho96@gmail•com>
To: Patrick Steinhardt <ps@pks•im>
Cc: "Seonghyeon Cho (조성현) via GitGitGadget" <gitgitgadget@gmail•com>,
git@vger•kernel.org
Subject: Re: [PATCH] add-interactive: reject malformed numerical input
Date: Sun, 7 Sep 2025 21:24:09 +0900 [thread overview]
Message-ID: <aL15aTmKQOsNrF0D@Seonghyeons-Mac-Studio.local> (raw)
In-Reply-To: <aLaz7yCXWGG2_oP_@pks.im>
On Tue, Sep 02, 2025 at 11:07:59AM +0200, Patrick Steinhardt wrote:
> Okay, that does feel fishy indeed. It would be good though to have a
> test case that demonstrates the new behaviour and at the same time
> ensures that we don't regress in the future. You can have a look at
> "t3701-add-interactive.sh", which has a bunch of other tests for this
> command, as well.
Okay, I'll add tests.
> In general though we're not doing a good job here of error checking. We
> don't at all verify whether `strtoul()` returned an error, for example
> ERANGE. So if a user passes an integer that exceeds whatever we can
> store in an `unsigned long` we'll silently proceed with a bogus result,
> won't we?
>
> Ideally, we'd use a saner interface to parse these integers, like for
> example our own `git_parse_ulong()`. But unfortunately, that interface
> does not handle the case where we only want to parse a substring in a
> longer string. Too bad.
Good point. Would you prefer I introduce new parse method here, or
should this be handled in separate patch?
> Coding style: the `else` should sit on the same line as the closing
> curly brace. And furthermore, if one of the branches of an if-else chain
> requires curly braces, then all branches should have curly braces.
Ok, I'll fix coding styles.
Thanks,
Seonghyeon
next prev parent reply other threads:[~2025-09-07 12:24 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-30 11:31 [PATCH] add-interactive: reject malformed numerical input Seonghyeon Cho (조성현) via GitGitGadget
2025-09-02 9:07 ` Patrick Steinhardt
2025-09-07 12:24 ` Seonghyeon Cho [this message]
2025-09-08 4:04 ` Patrick Steinhardt
2025-09-08 13:31 ` Seonghyeon Cho
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=aL15aTmKQOsNrF0D@Seonghyeons-Mac-Studio.local \
--to=seonghyeoncho96@gmail$(echo .)com \
--cc=aLaz7yCXWGG2_oP_@pks$(echo .)im \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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