diff: return const char from output_prefix callback

The diff_options structure has an output_prefix callback for returning a
prefix string, but it does so by returning a pointer to a strbuf.

This makes the interface awkward. There's no reason the callback should
need to use a strbuf, and it creates questions about whether the
ownership of the resulting buffer should be transferred to the caller
(it should not be, but a recent attempt to clean up this code led to a
double-free in some cases).

The one advantage we get is that the strbuf contains a ptr/len pair, so
we could in theory have a prefix with embedded NULs. But we can observe
that none of the existing callbacks would ever produce such a NUL (they
are usually just indentation or graph symbols, and even the
"--line-prefix" option takes a NUL-terminated string).

And anyway, only one caller (the one in log_tree_diff_flush) actually
looks at the strbuf length. In every other case we use a helper function
which discards the length and just returns the NUL-terminated string.

So let's just have the callback return a "const char *" pointer. It's up
to the callbacks themselves if they want to use a strbuf under the hood.
And now the caller in log_tree_diff_flush() can just use the helper
function along with everybody else. That lets us even simplify out the
function pointer check, since the helper returns an empty string
(technically this does mean we'll sometimes issue an empty fputs() call,
but I don't think this code path is hot enough to care about that).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff-lib.c b/diff-lib.c
index 7a1eb637579c7df952394b45b269f5519c435225..1cbf983a9c470e89fa29153f41b681506f45400a 100644 (file)
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -704,7 +704,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;
 }
@@ -719,7 +719,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 f725d217de7a29f1d361c4c2d7622a2a47a36675..a7bfe73fe6e72e9b8552d501305e29351c8fb6e2 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -2313,12 +2313,9 @@ 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 "";
-
-       msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
-       return msgbuf->buf;
+       return opt->output_prefix ?
+               opt->output_prefix(opt, opt->output_prefix_data) :
+               "";
 }
 
 static unsigned long sane_truncate_line(char *line, unsigned long len)

diff --git a/diff.h b/diff.h
index f816d3b12bf868c7ee6d03078f442d5f368db14e..ed91c92e3a274d94ac35f8975e4215050811ba63 100644 (file)
--- 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 34b18a80d48d483864d8c8de6c71328960093894..6a4db229bb8380e6873540717ee47d2218819ced 100644 (file)
--- a/graph.c
+++ b/graph.c
@@ -309,7 +309,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;
@@ -321,7 +321,7 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void
                strbuf_addstr(&msgbuf, opt->line_prefix);
        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 13524bc888186aabcf3caff2d92106203007211e..7a5828ce637e231c5cb9de0498a0868643d75206 100644 (file)
--- a/log-tree.c
+++ b/log-tree.c
@@ -922,12 +922,7 @@ int log_tree_diff_flush(struct rev_info *opt)
                         * diff/diffstat output for readability.
                         */
                        int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
-                       if (opt->diffopt.output_prefix) {
-                               struct strbuf *msg = NULL;
-                               msg = opt->diffopt.output_prefix(&opt->diffopt,
-                                       opt->diffopt.output_prefix_data);
-                               fwrite(msg->buf, msg->len, 1, opt->diffopt.file);
-                       }
+                       fputs(diff_line_prefix(&opt->diffopt), opt->diffopt.file);
 
                        /*
                         * We may have shown three-dashes line early

diff --git a/range-diff.c b/range-diff.c
index 5f01605550e57206accbf166a782681db4087eef..f28a388fb867865c9d77770a03b93919452711b1 100644 (file)
--- a/range-diff.c
+++ b/range-diff.c
@@ -478,7 +478,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;
 }
@@ -506,7 +506,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);
 
        /*