From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 1/2] ident: stop assuming that `gw_gecos` is writable
Date: Thu, 27 Feb 2025 13:15:31 -0800 [thread overview]
Message-ID: <xmqqwmdb5bt8.fsf@gitster.g> (raw)
In-Reply-To: <045c11dc1d51690eed4dc07da26dcace612f786c.1740671049.git.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Thu, 27 Feb 2025 15:44:08 +0000")
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In 590e081dea7c (ident: add NO_GECOS_IN_PWENT for systems without
> pw_gecos in struct passwd, 2011-05-19), code was introduced to iterate
> over the `gw_gecos` field; The loop variable is of type `char *`, which
> assumes that `gw_gecos` is writable.
>
> However, it is not necessarily writable (and it is a bad idea to have it
> writable in the first place), so let's switch the loop variable type to
> `const char *`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> ident.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Can we have a bit more details here (not in the commit but after the
three-dash line, meant for context) why the change from 2011 needs a
"hotfix" today? Has something in the build environment change? An
updated header file has changed definition for "struct passwd", or
something silly like that? Even though POSIX.1 does not define
gecos in its struct passwd in <pwd.h>, other string members like
pw_name and pw_dir are writable, which I found funny, and makes me
puzzled why this code from 2011 needs a hotfix all of the sudden.
> diff --git a/ident.c b/ident.c
> index caf41fb2a98..967895d8850 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
>
> static void copy_gecos(const struct passwd *w, struct strbuf *name)
> {
> - char *src;
> + const char *src;
>
> /* Traditionally GECOS field had office phone numbers etc, separated
> * with commas. Also & stands for capitalized form of the login name.
The patch text itself looks perfectly fine, so I am not opposed to
its eventual application. Even though we do declare "src" as
non-const, we only use it to read from it, so declaring it as const
pointer is perfectly fine and more appropriate.
But I do not see any urgency relative to Git 2.48.0 (or Git 2.48.1
for that matter) to mark this as "hotfix" implying that it should be
included in 2.49-rc1 in either the patch or the proposed commit log
message.
Thanks.
next prev parent reply other threads:[~2025-02-27 21:15 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-27 15:44 [PATCH 0/2] Hot fixes from Git for Windows v2.49.0-rc0 Johannes Schindelin via GitGitGadget
2025-02-27 15:44 ` [PATCH 1/2] ident: stop assuming that `gw_gecos` is writable Johannes Schindelin via GitGitGadget
2025-02-27 21:15 ` Junio C Hamano [this message]
2025-02-27 15:44 ` [PATCH 2/2] meson: fix sorting Johannes Schindelin via GitGitGadget
2025-02-28 7:58 ` Patrick Steinhardt
2025-03-06 10:26 ` [PATCH v2 0/3] Hot fixes from Git for Windows v2.49.0-rc0 Johannes Schindelin via GitGitGadget
2025-03-06 10:26 ` [PATCH v2 1/3] ident: stop assuming that `gw_gecos` is writable Johannes Schindelin via GitGitGadget
2025-03-06 10:50 ` Patrick Steinhardt
2025-03-06 12:26 ` Johannes Schindelin
2025-03-06 16:33 ` Junio C Hamano
2025-03-07 10:02 ` Patrick Steinhardt
2025-03-07 18:20 ` Junio C Hamano
2025-03-06 10:26 ` [PATCH v2 2/3] meson: fix sorting Johannes Schindelin via GitGitGadget
2025-03-06 10:26 ` [PATCH v2 3/3] cmake: generalize the handling of the `CLAR_TEST_OBJS` list Johannes Schindelin via GitGitGadget
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=xmqqwmdb5bt8.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johannes.schindelin@gmx.de \
/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;
as well as URLs for NNTP newsgroup(s).