Re: [PATCH v2] format-patch: allow --rfc to optionally take a value, like --rfc=WIP

From

Junio C Hamano <gitster@pobox.com>

Date

Apr 21, 2024, 18:58 UTC

Message-ID

<xmqqle561r1f.fsf@gitster.g>

In-Reply-To

<440ee2b7-2ed8-4c8b-96e8-623ef45efe42@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

>> +	struct strbuf rfc = STRBUF_INIT;
>
> Looking at this again, do we really need an strbuf here? I think we
> could we use "const char *" instead as we always store a static string
> in it which would avoid a memory leak from not calling
> strbuf_release().

Thanks for spotting the leak ;-)

Should be sufficient to squash in something like this.

 builtin/log.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git c/builtin/log.c w/builtin/log.c
index 0e9c84e51d..5c1c6f9b15 100644
--- c/builtin/log.c
+++ w/builtin/log.c
@@ -1497,12 +1497,11 @@ static int subject_prefix_callback(const struct option *opt, const char *arg,
 static int rfc_callback(const struct option *opt, const char *arg,
 			int unset)
 {
-	struct strbuf *rfc;
+	const char **rfc = opt->value;
 
-	rfc = opt->value;
-	strbuf_reset(rfc);
+	*rfc = opt->value;
 	if (!unset)
-		strbuf_addstr(rfc, arg ? arg : "RFC");
+		*rfc = arg ? arg : "RFC";
 	return 0;
 }
 
@@ -1919,7 +1918,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	struct strbuf sprefix = STRBUF_INIT;
-	struct strbuf rfc = STRBUF_INIT;
+	const char *rfc = NULL;
 	int creation_factor = -1;
 
 	const struct option builtin_format_patch_options[] = {
@@ -2064,8 +2063,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (cover_from_description_arg)
 		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
 
-	if (rfc.len)
-		strbuf_insertf(&sprefix, 0, "%s ", rfc.buf);
+	if (rfc)
+		strbuf_insertf(&sprefix, 0, "%s ", rfc);
 
 	if (reroll_count) {
 		strbuf_addf(&sprefix, " v%s", reroll_count);