From: "Jeff King via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, ps@pks.im, Jeff King <peff@peff.net>,
Derrick Stolee <stolee@gmail.com>, Jeff King <peff@peff.net>
Subject: [PATCH v2 3/3] diff: modify output_prefix function pointer
Date: Thu, 03 Oct 2024 11:58:44 +0000 [thread overview]
Message-ID: <e1d825ad212d91505eee9d911abbd3ba6bc170b1.1727956724.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1806.v2.git.1727956724.gitgitgadget@gmail.com>
From: Jeff King <peff@peff.net>
The uses of the output_prefix function pointer in the diff_options
struct is currently difficult to work with by returning a pointer to a
strbuf. There is only one use that cares about the length of the string,
which appears to be the only justification of the return type.
We already noticed confusing memory issues around this return type, so
use a const char * return type to make it clear that the caller does not
own this string buffer.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
diff-lib.c | 4 ++--
diff.c | 8 +++-----
diff.h | 2 +-
graph.c | 4 ++--
log-tree.c | 4 ++--
range-diff.c | 4 ++--
6 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index a680768ee75..6b14b959629 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -701,7 +701,7 @@ int index_differs_from(struct repository *r,
return (has_changes != 0);
}
-static struct strbuf *idiff_prefix_cb(struct diff_options *opt UNUSED, void *data)
+static const char *idiff_prefix_cb(struct diff_options *opt UNUSED, void *data)
{
return data;
}
@@ -716,7 +716,7 @@ void show_interdiff(const struct object_id *oid1, const struct object_id *oid2,
opts.output_format = DIFF_FORMAT_PATCH;
opts.output_prefix = idiff_prefix_cb;
strbuf_addchars(&prefix, ' ', indent);
- opts.output_prefix_data = &prefix;
+ opts.output_prefix_data = prefix.buf;
diff_setup_done(&opts);
diff_tree_oid(oid1, oid2, "", &opts);
diff --git a/diff.c b/diff.c
index 84a6bb08681..82a9545afbd 100644
--- a/diff.c
+++ b/diff.c
@@ -2315,12 +2315,10 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
const char *diff_line_prefix(struct diff_options *opt)
{
- struct strbuf *msgbuf;
- if (!opt->output_prefix)
- return "";
+ if (opt->output_prefix)
+ return opt->output_prefix(opt, opt->output_prefix_data);
- msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
- return msgbuf->buf;
+ return "";
}
static unsigned long sane_truncate_line(char *line, unsigned long len)
diff --git a/diff.h b/diff.h
index fb40c6e6d60..978450e20ac 100644
--- a/diff.h
+++ b/diff.h
@@ -94,7 +94,7 @@ typedef void (*add_remove_fn_t)(struct diff_options *options,
typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
struct diff_options *options, void *data);
-typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);
+typedef const char *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);
#define DIFF_FORMAT_RAW 0x0001
#define DIFF_FORMAT_DIFFSTAT 0x0002
diff --git a/graph.c b/graph.c
index 091c14cf4fb..ebb7d1e66f4 100644
--- a/graph.c
+++ b/graph.c
@@ -314,7 +314,7 @@ struct git_graph {
unsigned short default_column_color;
};
-static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void *data)
+static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
{
struct git_graph *graph = data;
static struct strbuf msgbuf = STRBUF_INIT;
@@ -327,7 +327,7 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void
opt->line_prefix_length);
if (graph)
graph_padding_line(graph, &msgbuf);
- return &msgbuf;
+ return msgbuf.buf;
}
static const struct diff_options *default_diffopt;
diff --git a/log-tree.c b/log-tree.c
index 3758e0d3b8e..3af34b91a59 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -923,10 +923,10 @@ int log_tree_diff_flush(struct rev_info *opt)
*/
int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
if (opt->diffopt.output_prefix) {
- struct strbuf *msg = NULL;
+ const char *msg;
msg = opt->diffopt.output_prefix(&opt->diffopt,
opt->diffopt.output_prefix_data);
- fwrite(msg->buf, msg->len, 1, opt->diffopt.file);
+ fwrite(msg, strlen(msg), 1, opt->diffopt.file);
}
/*
diff --git a/range-diff.c b/range-diff.c
index bbb0952264b..10885ba3013 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -480,7 +480,7 @@ static void patch_diff(const char *a, const char *b,
diff_flush(diffopt);
}
-static struct strbuf *output_prefix_cb(struct diff_options *opt UNUSED, void *data)
+static const char *output_prefix_cb(struct diff_options *opt UNUSED, void *data)
{
return data;
}
@@ -508,7 +508,7 @@ static void output(struct string_list *a, struct string_list *b,
opts.flags.suppress_hunk_header_line_count = 1;
opts.output_prefix = output_prefix_cb;
strbuf_addstr(&indent, " ");
- opts.output_prefix_data = &indent;
+ opts.output_prefix_data = indent.buf;
diff_setup_done(&opts);
/*
--
gitgitgadget
next prev parent reply other threads:[~2024-10-03 11:58 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-02 16:07 [PATCH] line-log: protect inner strbuf from free Derrick Stolee via GitGitGadget
2024-10-02 23:56 ` Jeff King
2024-10-03 0:19 ` Jeff King
2024-10-03 2:36 ` Derrick Stolee
2024-10-03 6:11 ` Jeff King
2024-10-03 12:23 ` Derrick Stolee
2024-10-03 21:02 ` Jeff King
2024-10-03 11:58 ` [PATCH v2 0/3] " Derrick Stolee via GitGitGadget
2024-10-03 11:58 ` [PATCH v2 1/3] " Derrick Stolee via GitGitGadget
2024-10-04 4:32 ` Patrick Steinhardt
2024-10-03 11:58 ` [PATCH v2 2/3] line-log: remove output_prefix() Jeff King via GitGitGadget
2024-10-03 11:58 ` Jeff King via GitGitGadget [this message]
2024-10-03 16:26 ` [PATCH v2 3/3] diff: modify output_prefix function pointer Junio C Hamano
2024-10-03 21:05 ` [PATCH 0/5] diff output_prefix cleanups Jeff King
2024-10-03 21:06 ` [PATCH 1/5] line-log: use diff_line_prefix() instead of custom helper Jeff King
2024-10-03 21:06 ` [PATCH 2/5] diff: drop line_prefix_length field Jeff King
2024-10-03 21:09 ` [PATCH 3/5] diff: return const char from output_prefix callback Jeff King
2024-10-03 21:11 ` [PATCH 4/5] diff: return line_prefix directly when possible Jeff King
2024-10-03 21:13 ` [PATCH 5/5] diff: store graph prefix buf in git_graph struct Jeff King
2024-10-03 23:43 ` Junio C Hamano
2024-10-04 4:32 ` Patrick Steinhardt
2024-10-04 19:27 ` [PATCH 0/5] diff output_prefix cleanups Derrick Stolee
2024-10-04 19:31 ` Junio C Hamano
2024-10-04 19:33 ` Derrick Stolee
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=e1d825ad212d91505eee9d911abbd3ba6bc170b1.1727956724.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
--cc=stolee@gmail.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).