From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Hamza Mahfooz <someguy@effective-light.com>,
Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH 6/5] grep.c: mark eol/bol and derived as "const char * const"
Date: Tue, 21 Sep 2021 17:17:57 +0200 [thread overview]
Message-ID: <874kadapq1.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <YUnx7gt0KQNRlhuZ@coredump.intra.peff.net>
On Tue, Sep 21 2021, Jeff King wrote:
> On Tue, Sep 21, 2021 at 02:45:16PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> I think that generally git's codebase could use going beyond just
>> "const char *" when a "const char * const" would suffice, for some
>> reason we seem to mostly use it for the static usage variables.
>
> I didn't dig up the references in the list archive, but I feel like
> we've had this discussion long ago. One of the reasons not to do so is
> that it pollutes the function's interface with internal details.[...]
Are there cases in my conversion where the caller has to do anything
special that they didn't before? These are also all static functions, so
it's all internal details exported to nobody.
> The caller does not care whether the function is going to modify the
> pointer itself, because it is passed by value.
Sure, it's for increased clarity of reading te function in question, not
its although in one case we pass a ** so you can see what exactly we
modify both from the callers and function perspective.
> You could apply the same logic that we should be passing "const int",
> and so on.
Sure, in general I think churn like that isn't worth it, and "const char
*" is usually good enough, even if it could be "const char * const" or
whatever.
I think it makes senes in this specific case where you need to read one
function after another with "bol" and "eol" variables, with some
treating their copy as immutable, others not etc. Particularly if we'd
convert it to some other style (e.g. str/len) we can see if an entire
chain of functions can all be safely changed over.
next prev parent reply other threads:[~2021-09-21 15:56 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-21 3:45 [PATCH 0/5] const-correctness in grep.c Jeff King
2021-09-21 3:46 ` [PATCH 1/5] grep: stop modifying buffer in strip_timestamp Jeff King
2021-09-21 5:18 ` Carlo Arenas
2021-09-21 5:24 ` Eric Sunshine
2021-09-21 5:40 ` Carlo Arenas
2021-09-21 5:43 ` Jeff King
2021-09-21 6:42 ` Carlo Marcelo Arenas Belón
2021-09-21 7:37 ` René Scharfe
2021-09-21 14:24 ` Jeff King
2021-09-21 21:02 ` Ævar Arnfjörð Bjarmason
2021-09-22 20:20 ` Jeff King
2021-09-23 0:53 ` Ævar Arnfjörð Bjarmason
2021-09-21 3:48 ` [PATCH 2/5] grep: stop modifying buffer in show_line() Jeff King
2021-09-21 4:22 ` Taylor Blau
2021-09-21 4:42 ` Jeff King
2021-09-21 4:45 ` Taylor Blau
2021-09-21 3:48 ` [PATCH 3/5] grep: stop modifying buffer in grep_source_1() Jeff King
2021-09-21 3:49 ` [PATCH 4/5] grep: mark "haystack" buffers as const Jeff King
2021-09-21 12:04 ` Ævar Arnfjörð Bjarmason
2021-09-21 14:27 ` Jeff King
2021-09-21 3:51 ` [PATCH 5/5] grep: store grep_source buffer " Jeff King
2021-09-21 4:30 ` [PATCH 0/5] const-correctness in grep.c Taylor Blau
2021-09-21 12:07 ` Ævar Arnfjörð Bjarmason
2021-09-21 14:49 ` Jeff King
2021-09-21 12:45 ` [PATCH 6/5] grep.c: mark eol/bol and derived as "const char * const" Ævar Arnfjörð Bjarmason
2021-09-21 14:53 ` Jeff King
2021-09-21 15:17 ` Ævar Arnfjörð Bjarmason [this message]
2021-09-21 19:18 ` Jeff King
2021-09-23 13:56 ` Ævar Arnfjörð Bjarmason
2021-09-24 4:22 ` Junio C Hamano
2021-09-22 19:02 ` Junio C Hamano
2021-09-22 18:57 ` [PATCH 0/5] const-correctness in grep.c Junio C Hamano
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=874kadapq1.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
--cc=someguy@effective-light.com \
/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).