From: Junio C Hamano <gitster@pobox•com>
To: "Jean-Noël Avila" <avila.jn@gmail•com>
Cc: Atneya Nair <atneya@google•com>,
git@vger•kernel.org, jeffhost@microsoft•com, me@ttaylorr•com,
nasamuffin@google•com
Subject: Re: [RFC PATCH 1/3] Make read_gitfile and resolve_gitfile thread safe
Date: Wed, 06 Mar 2024 08:57:01 -0800 [thread overview]
Message-ID: <xmqqttljs3he.fsf@gitster.g> (raw)
In-Reply-To: <10042df8-5d06-47cd-9202-ea6965f50784@gmail.com> ("Jean-Noël Avila"'s message of "Wed, 6 Mar 2024 09:13:15 +0100")
Jean-Noël Avila <avila.jn@gmail•com> writes:
>> -const char *read_gitfile_gently(const char *path, int *return_error_code)
>> +const char *read_gitfile_gently(const char *path, int *return_error_code, struct strbuf* result_buf)
>> {
>> const int max_file_size = 1 << 20; /* 1MB */
>> int error_code = 0;
>> @@ -848,7 +852,10 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
>> struct stat st;
>> int fd;
>> ssize_t len;
>> - static struct strbuf realpath = STRBUF_INIT;
>> + static struct strbuf shared = STRBUF_INIT;
>> + if (!result_buf) {
>> + result_buf = &shared;
>> + }
>>
>
> Question about general style: is it accepted practice to override a
> parameter of a function?
We do not forbid it. We have a rule against enclosing a single
statement block inside {braces}, though ;-).
> I would have written:
If it matters to know what the caller-supplied value of the
parameter was, then we would probably write that way. If it does
not, then the above is perfectly fine. Even with the above, if a
later code really wanted to, it can compare the pointers to find out
if the caller was uninterested in the result (i.e., passed NULL),
but at that point, we may be better off to (re)write it your way.
>> - strbuf_realpath(&realpath, dir, 1);
>> - path = realpath.buf;
>> + strbuf_realpath(result_buf, dir, 1);
>> + path = result_buf->buf;
>> + // TODO is this valid?
>> + if (!path) die(_("Unexpected null from realpath '%s'"), dir);
>
> In fact, this is not a null path, but an empty path (null is not part of
> the string).
> By the way, shouldn't this be an internal bug instead of a message to
> the user?
Unless the strbuf instance the result_buf pointer points at is
corrupt, its .buf member should *NEVER* be NULL. Testing for NULL
is meaningless, unless you are manually futzing with the members of
strbuf (you shouldn't).
Thanks for carefully reading.
next prev parent reply other threads:[~2024-03-06 16:57 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-05 1:21 [RFC PATCH 0/3] Parallel submodule status Atneya Nair
2024-03-05 1:21 ` [RFC PATCH 1/3] Make read_gitfile and resolve_gitfile thread safe Atneya Nair
2024-03-05 2:22 ` Junio C Hamano
2024-03-05 4:29 ` Eric Sunshine
2024-03-12 20:38 ` Atneya Nair
2024-03-06 8:13 ` Jean-Noël Avila
2024-03-06 16:57 ` Junio C Hamano [this message]
2024-03-12 20:44 ` Atneya Nair
2024-03-05 1:21 ` [RFC PATCH 2/3] Make ce_compare_gitlink thread-safe Atneya Nair
2024-03-05 17:08 ` Junio C Hamano
2024-03-05 18:53 ` Junio C Hamano
2024-03-06 1:23 ` Jeff King
2024-03-06 1:58 ` Junio C Hamano
2024-03-12 22:13 ` Atneya Nair
2024-03-12 22:15 ` Atneya Nair
2024-03-13 17:51 ` Junio C Hamano
2024-03-05 1:21 ` [RFC PATCH 3/3] Preload submodule state in refresh_index Atneya Nair
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=xmqqttljs3he.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=atneya@google$(echo .)com \
--cc=avila.jn@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jeffhost@microsoft$(echo .)com \
--cc=me@ttaylorr$(echo .)com \
--cc=nasamuffin@google$(echo .)com \
/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