From: <rsbecker@nexbridge.com>
To: "'Mohit Marathe'" <mohitmarathe@proton.me>,
"'Christian Couder'" <christian.couder@gmail.com>
Cc: <git@vger.kernel.org>, <gitster@pobox.com>,
<britton.kerin@gmail.com>, <peff@peff.net>
Subject: RE: [GSoC][RFC] Replace use of atoi() with strtol_i(), as a microproject
Date: Wed, 10 Jan 2024 12:55:11 -0500 [thread overview]
Message-ID: <004601da43ee$2b6cd0c0$82467240$@nexbridge.com> (raw)
In-Reply-To: <F6ejgAfr2IMRNR3Tq0CDTHeT9xMWzJ9ley8M_fnSX97ayRNRp_CEgA62WdtOooi9bha1WJPGB53ptJYQFII2lCbIflwgNvbIaefw7nK8w7M=@proton.me>
On Wednesday, January 10, 2024 12:38 PM, Mohit Marathe wrote:
>>In https://public-inbox.org/git/xmqqjzpjsbjl.fsf@gitster.g/ Junio says:
>>
>>"Some places use atoi() immediately followed by strspn() to skip over
>>digits, which means they are parsing an integer and want to continue
>>reading after the integer, which is incompatible with what
>>strtol_i() wants to do. They need either a separate helper or an
>>updated strtol_i() that optionally allows you to parse the prefix and
>>report where the integer ended, e.g., something like:"
>>
>>and then he suggests the above helper.
>>
>>So it seems that the two instances you found look like good places
>>where Junio says the new helper could be useful.
>>
>>Now if you want to continue further on this, I think you would need to
>>take a closer look at those two instances to see if replacing atoi()
>>there with the new helper would improve something there or not. If you
>>find it would improve something, be sure to explain what would be
>>improved in the commit message.
>
>I took a closer look at `builtin/patch-id.c` and it seems replacing `atoi()` (which is
>used to parse numbers in the hunk header) wouldn't improve anything, unless I'm
>missing something.
>
>So then I tried finding other places where `atoi()` can be replaced but I find it
>difficult to find any reason that would justify the change.
>So far I've only looked at few of the MANY occurrences of `atoi()`.
>As far as I understand, the only advantage of `strtol_i()` over `atoi()` is better error
>handling. And most of places I've seen either already takes care of that or does not
>need that at all.
I am not sure this is a good idea. The error detection inside strtol_i() reports a -1 if the supplied text value is invalid. This does not differentiate between an invalid value and a valid "-1" supplied. Replacing all instances of atoi() with strtol_i() will likely cause breakages as the error semantics are different between the two.
--Randall
next prev parent reply other threads:[~2024-01-10 18:04 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-08 17:34 [GSoC][RFC] Replace use of atoi() with strtol_i(), as a microproject mohitmarathe
2024-01-09 9:56 ` Christian Couder
2024-01-10 17:38 ` Mohit Marathe
2024-01-10 17:55 ` rsbecker [this message]
2024-01-10 18:46 ` Junio C Hamano
2024-01-10 18:44 ` Junio C Hamano
2024-01-12 13:04 ` Mohit Marathe
2024-01-17 5:28 ` Mohit Marathe
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='004601da43ee$2b6cd0c0$82467240$@nexbridge.com' \
--to=rsbecker@nexbridge.com \
--cc=britton.kerin@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mohitmarathe@proton.me \
--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 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.