threads / patch / 54646

patcharchive: release refname after use

Subject: [PATCH] archive: release refname after use

## tl;dr

2 messages between Nov 14, 2020 and Nov 17, 2020. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

René Scharfe· Nov 14, 2020, 22:01 UTC · lore

parse_treeish_arg() uses dwim_ref() to set refname to a strdup'd string. Release it after use. Also remove the const qualifier from the refname member to signify that ownership of the string is handed to the struct, leaving cleanup duty with the caller of parse_treeish_arg(), thus avoiding a cast.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 archive.c | 1 +
 archive.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Show changes to 2 files +2 −2

archive.c, archive.h

diff --git a/archive.c b/archive.c
index 3c1541af9e..da89d292c2 100644
--- a/archive.c
+++ b/archive.c
@@ -662,6 +662,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	rc = ar->write_archive(ar, &args);

 	string_list_clear_func(&args.extra_files, extra_file_info_clear);
+	free(args.refname);

 	return rc;
 }
diff --git a/archive.h b/archive.h
index 82b226011a..43028ffbf3 100644
--- a/archive.h
+++ b/archive.h
@@ -8,7 +8,7 @@ struct repository;

 struct archiver_args {
 	struct repository *repo;
-	const char *refname;
+	char *refname;
 	const char *prefix;
 	const char *base;
 	size_t baselen;
--
2.29.2

Jeff King· Nov 17, 2020, 00:28 UTC · re: René Scharfe · lore

Re: [PATCH] archive: release refname after use

On Sat, Nov 14, 2020 at 11:01:04PM +0100, René Scharfe wrote:

Show 5 quoted lines

> parse_treeish_arg() uses dwim_ref() to set refname to a strdup'd string.
> Release it after use.  Also remove the const qualifier from the refname
> member to signify that ownership of the string is handed to the struct,
> leaving cleanup duty with the caller of parse_treeish_arg(), thus
> avoiding a cast.

Yeah, I looked at the surrounding code and agree that this is the right thing to do.

I wondered if it might make sense for the initialization and clearing of this "archive_args" structure to go in a pair of matched functions, but it probably isn't worth the trouble. write_archive() is the only place where we will allocate and deallocate such a struct (the only reason it is a type at all is to pass the set of values around the stack).

-Peff