[PATCH] Make remaining usage strings and string tables const

[PATCH] Make remaining usage strings and string tables const

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 9745 bytes --]

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 builtin/blame.c                    | 10 +++++-----
 builtin/commit-graph.c             |  4 ++--
 builtin/fast-export.c              |  2 +-
 builtin/mktree.c                   |  2 +-
 builtin/pack-objects.c             |  2 +-
 builtin/show-branch.c              |  2 +-
 t/helper/test-bloom.c              |  2 +-
 t/helper/test-date.c               |  2 +-
 t/helper/test-find-pack.c          |  2 +-
 t/helper/test-getcwd.c             |  2 +-
 t/helper/test-pack-mtimes.c        |  2 +-
 t/helper/test-proc-receive.c       |  2 +-
 t/helper/test-rot13-filter.c       |  2 +-
 t/helper/test-submodule.c          | 10 +++++-----
 t/helper/test-windows-named-pipe.c |  2 +-
 15 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 9436f70aec..682cb0e77b 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -36,17 +36,17 @@
 #include "tag.h"
 #include "write-or-die.h"
 
-static char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>");
-static char annotate_usage[] = N_("git annotate [<options>] [<rev-opts>] [<rev>] [--] <file>");
+static const char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>");
+static const char annotate_usage[] = N_("git annotate [<options>] [<rev-opts>] [<rev>] [--] <file>");
 
-static const char *blame_opt_usage[] = {
+static const char *const blame_opt_usage[] = {
 	blame_usage,
 	"",
 	N_("<rev-opts> are documented in git-rev-list(1)"),
 	NULL
 };
 
-static const char *annotate_opt_usage[] = {
+static const char *const annotate_opt_usage[] = {
 	annotate_usage,
 	"",
 	N_("<rev-opts> are documented in git-rev-list(1)"),
@@ -944,7 +944,7 @@ int cmd_blame(int argc,
 	long anchor;
 	long num_lines = 0;
 	const char *str_usage = cmd_is_annotate ? annotate_usage : blame_usage;
-	const char **opt_usage = cmd_is_annotate ? annotate_opt_usage : blame_opt_usage;
+	const char *const *opt_usage = cmd_is_annotate ? annotate_opt_usage : blame_opt_usage;
 
 	setup_default_color_by_age();
 	git_config(git_blame_config, &output_option);
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 8ca75262c5..a1c1af7f3f 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -22,12 +22,12 @@
 	   "                       [--changed-paths] [--[no-]max-new-filters <n>] [--[no-]progress]\n" \
 	   "                       <split-options>")
 
-static const char * builtin_commit_graph_verify_usage[] = {
+static const char * const builtin_commit_graph_verify_usage[] = {
 	BUILTIN_COMMIT_GRAPH_VERIFY_USAGE,
 	NULL
 };
 
-static const char * builtin_commit_graph_write_usage[] = {
+static const char * const builtin_commit_graph_write_usage[] = {
 	BUILTIN_COMMIT_GRAPH_WRITE_USAGE,
 	NULL
 };
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 170126d41a..f2f7586418 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -30,7 +30,7 @@
 #include "remote.h"
 #include "blob.h"
 
-static const char *fast_export_usage[] = {
+static const char *const fast_export_usage[] = {
 	N_("git fast-export [<rev-list-opts>]"),
 	NULL
 };
diff --git a/builtin/mktree.c b/builtin/mktree.c
index 3c16faa40e..22486963af 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -66,7 +66,7 @@ static void write_tree(struct object_id *oid)
 	strbuf_release(&buf);
 }
 
-static const char *mktree_usage[] = {
+static const char *const mktree_usage[] = {
 	"git mktree [-z] [--missing] [--batch]",
 	NULL
 };
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3973267e9e..91bb1260e9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -183,7 +183,7 @@ static inline void oe_set_delta_size(struct packing_data *pack,
 #define SET_DELTA_CHILD(obj, val) oe_set_delta_child(&to_pack, obj, val)
 #define SET_DELTA_SIBLING(obj, val) oe_set_delta_sibling(&to_pack, obj, val)
 
-static const char *pack_usage[] = {
+static const char *const pack_usage[] = {
 	N_("git pack-objects --stdout [<options>] [< <ref-list> | < <object-list>]"),
 	N_("git pack-objects [<options>] <base-name> [< <ref-list> | < <object-list>]"),
 	NULL
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index fce6b404e9..c74b4df7ec 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -19,7 +19,7 @@
 #include "date.h"
 #include "wildmatch.h"
 
-static const char* show_branch_usage[] = {
+static const char*const show_branch_usage[] = {
     N_("git show-branch [-a | --all] [-r | --remotes] [--topo-order | --date-order]\n"
        "                [--current] [--color[=<when>] | --no-color] [--sparse]\n"
        "                [--more=<n> | --list | --independent | --merge-base]\n"
diff --git a/t/helper/test-bloom.c b/t/helper/test-bloom.c
index 14e075c1a1..9aa2c5a592 100644
--- a/t/helper/test-bloom.c
+++ b/t/helper/test-bloom.c
@@ -44,7 +44,7 @@ static void get_bloom_filter_for_commit(const struct object_id *commit_oid)
 	print_bloom_filter(filter);
 }
 
-static const char *bloom_usage = "\n"
+static const char *const bloom_usage = "\n"
 "  test-tool bloom get_murmur3 <string>\n"
 "  test-tool bloom get_murmur3_seven_highbit\n"
 "  test-tool bloom generate_filter <string> [<string>...]\n"
diff --git a/t/helper/test-date.c b/t/helper/test-date.c
index f25512de9a..87d2ad6fca 100644
--- a/t/helper/test-date.c
+++ b/t/helper/test-date.c
@@ -2,7 +2,7 @@
 #include "date.h"
 #include "trace.h"
 
-static const char *usage_msg = "\n"
+static const char *const usage_msg = "\n"
 "  test-tool date relative [time_t]...\n"
 "  test-tool date human [time_t]...\n"
 "  test-tool date show:<format> [time_t]...\n"
diff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c
index 85a69a4e55..76c2f4eba8 100644
--- a/t/helper/test-find-pack.c
+++ b/t/helper/test-find-pack.c
@@ -15,7 +15,7 @@
  * packfiles containing the object is not <n>.
  */
 
-static const char *find_pack_usage[] = {
+static const char *const find_pack_usage[] = {
 	"test-tool find-pack [--check-count <n>] <object>",
 	NULL
 };
diff --git a/t/helper/test-getcwd.c b/t/helper/test-getcwd.c
index d680038a78..cd4d424079 100644
--- a/t/helper/test-getcwd.c
+++ b/t/helper/test-getcwd.c
@@ -2,7 +2,7 @@
 #include "git-compat-util.h"
 #include "parse-options.h"
 
-static const char *getcwd_usage[] = {
+static const char *const getcwd_usage[] = {
 	"test-tool getcwd",
 	NULL
 };
diff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c
index f8f9afbb5b..fb8d919f6b 100644
--- a/t/helper/test-pack-mtimes.c
+++ b/t/helper/test-pack-mtimes.c
@@ -24,7 +24,7 @@ static void dump_mtimes(struct packed_git *p)
 	}
 }
 
-static const char *pack_mtimes_usage = "\n"
+static const char *const pack_mtimes_usage = "\n"
 "  test-tool pack-mtimes <pack-name.mtimes>";
 
 int cmd__pack_mtimes(int argc, const char **argv)
diff --git a/t/helper/test-proc-receive.c b/t/helper/test-proc-receive.c
index 3703f734f3..8eccc34216 100644
--- a/t/helper/test-proc-receive.c
+++ b/t/helper/test-proc-receive.c
@@ -6,7 +6,7 @@
 #include "sigchain.h"
 #include "string-list.h"
 
-static const char *proc_receive_usage[] = {
+static const char *const proc_receive_usage[] = {
 	"test-tool proc-receive [<options>]",
 	NULL
 };
diff --git a/t/helper/test-rot13-filter.c b/t/helper/test-rot13-filter.c
index 722b1cbe77..ad37e10034 100644
--- a/t/helper/test-rot13-filter.c
+++ b/t/helper/test-rot13-filter.c
@@ -324,7 +324,7 @@ static void packet_initialize(void)
 	packet_flush(1);
 }
 
-static const char *rot13_usage[] = {
+static const char *const rot13_usage[] = {
 	"test-tool rot13-filter [--always-delay] --log=<path> <capabilities>",
 	NULL
 };
diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
index 22e518d229..0133852e1e 100644
--- a/t/helper/test-submodule.c
+++ b/t/helper/test-submodule.c
@@ -12,33 +12,33 @@
 
 #define TEST_TOOL_CHECK_NAME_USAGE \
 	"test-tool submodule check-name"
-static const char *submodule_check_name_usage[] = {
+static const char *const submodule_check_name_usage[] = {
 	TEST_TOOL_CHECK_NAME_USAGE,
 	NULL
 };
 
 #define TEST_TOOL_CHECK_URL_USAGE \
 	"test-tool submodule check-url"
-static const char *submodule_check_url_usage[] = {
+static const char *const submodule_check_url_usage[] = {
 	TEST_TOOL_CHECK_URL_USAGE,
 	NULL
 };
 
 #define TEST_TOOL_IS_ACTIVE_USAGE \
 	"test-tool submodule is-active <name>"
-static const char *submodule_is_active_usage[] = {
+static const char *const submodule_is_active_usage[] = {
 	TEST_TOOL_IS_ACTIVE_USAGE,
 	NULL
 };
 
 #define TEST_TOOL_RESOLVE_RELATIVE_URL_USAGE \
 	"test-tool submodule resolve-relative-url <up_path> <remoteurl> <url>"
-static const char *submodule_resolve_relative_url_usage[] = {
+static const char *const submodule_resolve_relative_url_usage[] = {
 	TEST_TOOL_RESOLVE_RELATIVE_URL_USAGE,
 	NULL,
 };
 
-static const char *submodule_usage[] = {
+static const char *const submodule_usage[] = {
 	TEST_TOOL_CHECK_NAME_USAGE,
 	TEST_TOOL_CHECK_URL_USAGE,
 	TEST_TOOL_IS_ACTIVE_USAGE,
diff --git a/t/helper/test-windows-named-pipe.c b/t/helper/test-windows-named-pipe.c
index ae52183e63..bd73784ceb 100644
--- a/t/helper/test-windows-named-pipe.c
+++ b/t/helper/test-windows-named-pipe.c
@@ -3,7 +3,7 @@
 #include "strbuf.h"
 
 #ifdef GIT_WINDOWS_NATIVE
-static const char *usage_string = "<pipe-filename>";
+static const char *const usage_string = "<pipe-filename>";
 
 #define TEST_BUFSIZE (4096)
 
-- 
2.39.5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Make remaining usage strings and string tables const

[Junio C Hamano]

Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> writes:

> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---

I've looked at these changes (I wouldn't claim with fine toothed
comb, though) and they looked correct.  Most of the changes are to
ensure not just the strings are not writable through the pointers in
the arrays but the arrays themselves cannot be modified to hold
pionters that point elsewhere.

Functions like parse_revision_opt() declare their parameter for the
usage string array as

	const char *const usagestr[]

but that merely promises to the caller that the function would not
touch the strings or the pointers in the incoming array, so the
callers declaring the array they pass, like blame_opt_usage[] here,

> -static const char *blame_opt_usage[] = {
> +static const char *const blame_opt_usage[] = {
>  	blame_usage,
>  	"",
>  	N_("<rev-opts> are documented in git-rev-list(1)"),
>  	NULL
>  };

much looser to allow swapping the pointers in the array is *not* a
problem per-se, but as long as the compiler would not barf after
this patch, we know these callers that use these arrays are not
doing such mutations to these arrays themselves, so it is good.

Thanks.

Re: [PATCH] Make remaining usage strings and string tables const

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

On Mon, Apr 21, 2025 at 01:30:36PM -0700, Junio C Hamano wrote:
> Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> writes:
> > -static const char *blame_opt_usage[] = {
> > +static const char *const blame_opt_usage[] = {
> >  	blame_usage,
> >  	"",
> >  	N_("<rev-opts> are documented in git-rev-list(1)"),
> >  	NULL
> >  };
> much looser to allow swapping the pointers in the array is *not* a
> problem per-se, but as long as the compiler would not barf after
> this patch, we know these callers that use these arrays are not
> doing such mutations to these arrays themselves, so it is good.
This built for me on Debian and the resulting binary worked so there's
no obvious sneaky const-incorrectness; it only resulted in one error,
where one of these variables was assigned to a
  const char *      *opt_usage;
instead of a
  const char *const *opt_usage;
this has been obviously fixed and is part of this submission.

I came across this as I was debugging some other issue
(that ended up as me holding it wrong),
static const char *s in library functions appeared quite pervasive,
but those aren't really possible to identify and close semi-automatically
(and it's unclear to me if a tighter analysis scope allows the compiler
 to actually put the static in .rodata or eliminate it outright).

Best,

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]