[v2,4/7] var: adjust memory allocation for strings

Commit Message

brian m. carlson June 26, 2023, 7 p.m. UTC

From: "brian m. carlson" <[e-mail]>

Right now, all of our values are constants whose allocation is managed
elsewhere.  However, in the future, we'll have some variables whose
memory we will need to free.  To keep things consistent, let's make each
of our functions allocate its own memory and make the caller responsible
for freeing it.

Signed-off-by: brian m. carlson <[e-mail]>
---
 builtin/var.c | 45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

Comments

Junio C Hamano June 26, 2023, 7:56 p.m. UTC | #1

"brian m. carlson" <[e-mail]> writes:

> From: "brian m. carlson" <[e-mail]>
>
> Right now, all of our values are constants whose allocation is managed
> elsewhere.  However, in the future, we'll have some variables whose
> memory we will need to free.  To keep things consistent, let's make each
> of our functions allocate its own memory and make the caller responsible
> for freeing it.
>
> Signed-off-by: brian m. carlson <[e-mail]>
> ---
>  builtin/var.c | 45 +++++++++++++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 16 deletions(-)

Making everybody allocate like this patch does is also fine, but
FWIW, with [3/7], we can selectively set ".need_free = 1" for
minority of elements in the array that returns an allocated piece of
memory without making the source code too noisy (as the array
elements for existing majority can omit ".need_free = 0" and leave
it to the default initialization), so I am not opposed to the "we
set .need_free bit only for those that we allocate" approach taken
with the previous round.

Thanks.

Patch

diff --git a/builtin/var.c b/builtin/var.c
index bd2750b1bf..ce6a2231ca 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -12,47 +12,57 @@ 
 
 static const char var_usage[] = "git var (-l | <variable>)";
 
-static const char *editor(int flag)
+static char *committer(int flag)
 {
-	return git_editor();
+	return xstrdup_or_null(git_committer_info(flag));
 }
 
-static const char *sequence_editor(int flag)
+static char *author(int flag)
 {
-	return git_sequence_editor();
+	return xstrdup_or_null(git_author_info(flag));
 }
 
-static const char *pager(int flag)
+static char *editor(int flag)
+{
+	return xstrdup_or_null(git_editor());
+}
+
+static char *sequence_editor(int flag)
+{
+	return xstrdup_or_null(git_sequence_editor());
+}
+
+static char *pager(int flag)
 {
 	const char *pgm = git_pager(1);
 
 	if (!pgm)
 		pgm = "cat";
-	return pgm;
+	return xstrdup(pgm);
 }
 
-static const char *default_branch(int flag)
+static char *default_branch(int flag)
 {
-	return git_default_branch_name(1);
+	return xstrdup_or_null(git_default_branch_name(1));
 }
 
-static const char *shell_path(int flag)
+static char *shell_path(int flag)
 {
-	return SHELL_PATH;
+	return xstrdup(SHELL_PATH);
 }
 
 struct git_var {
 	const char *name;
-	const char *(*read)(int);
+	char *(*read)(int);
 };
 static struct git_var git_vars[] = {
 	{
 		.name = "GIT_COMMITTER_IDENT",
-		.read = git_committer_info,
+		.read = committer,
 	},
 	{
 		.name = "GIT_AUTHOR_IDENT",
-		.read = git_author_info,
+		.read = author,
 	},
 	{
 		.name = "GIT_EDITOR",
@@ -83,11 +93,13 @@  static struct git_var git_vars[] = {
 static void list_vars(void)
 {
 	struct git_var *ptr;
-	const char *val;
+	char *val;
 
 	for (ptr = git_vars; ptr->read; ptr++)
-		if ((val = ptr->read(0)))
+		if ((val = ptr->read(0))) {
 			printf("%s=%s\n", ptr->name, val);
+			free(val);
+		}
 }
 
 static const struct git_var *get_git_var(const char *var)
@@ -113,7 +125,7 @@  static int show_config(const char *var, const char *value, void *cb)
 int cmd_var(int argc, const char **argv, const char *prefix UNUSED)
 {
 	const struct git_var *git_var;
-	const char *val;
+	char *val;
 
 	if (argc != 2)
 		usage(var_usage);
@@ -134,6 +146,7 @@  int cmd_var(int argc, const char **argv, const char *prefix UNUSED)
 		return 1;
 
 	printf("%s\n", val);
+	free(val);
 
 	return 0;
 }