From patchwork Thu Oct 3 21:06:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13821582 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1EA011B85D6 for ; Thu, 3 Oct 2024 21:06:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989590; cv=none; b=GcGbk7KPk6hTxQwQ/dRZ1Q2heE0ewE7HthVP795DmWWWF3pJDZt/ssTuNQacJhxxfWcOV5fd5zFixmMNxsZTNvFyF6BJbaABAwkjzSLmPne5P6+BAk8wlER5kNNt+G1MDlxm+I8wEoxRenCFpCdT7qnxZ/1CF2U887Ng8HXZr68= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989590; c=relaxed/simple; bh=9eNMHCRMh+R0g/8wakjjaqeDTr2KNnTMOA9W2hKmJxQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=rNq3C1l/V1J5S8WNBUQ61hVD7lcpTkrl+0UWXwFRKx1tq8QWxpcfZEBP3Xp5cQwlvQYmb5zEcZkw55WkNAZAtjoesFwUg6edeL7oA5k61IvSH6mRYCzwGfuWFMac4LlDJTHNu7LPcoKdCtJwbf/K8QM0B8TKV1akz3LgbrJCTSI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=JY9qI46O; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="JY9qI46O" Received: (qmail 19317 invoked by uid 109); 3 Oct 2024 21:06:28 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=9eNMHCRMh+R0g/8wakjjaqeDTr2KNnTMOA9W2hKmJxQ=; b=JY9qI46OlQKABp72JUFMSSJST2j33DsQB33akmee1Cf0/+JKx1Z2adOZF299cDfCs8Xe84dVSbvyYL0Et97y4x4pSADmThfWLoVWdH89MhC3WMXwWHT0wucbmNXXDmOeed0YQsWjUdPutZwrn1THB9wjw+idurMiefGj3NLJqPW4uG/N1vfxdUGImJBtXZxW8ywENetzp7W0gfrf1OPISVzT/lAZ1fP96OwRaUSut/O3iImbAm0AWEUjJDE9qu4WOrtxc6yrH+nnMqJBB8Bm89pAOzjwI++2uq0lcULwONLEJVa95+NTBwrxbAkPvtaPmogSlEECxbIQ7yVKcq5tNw== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 03 Oct 2024 21:06:28 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 21755 invoked by uid 111); 3 Oct 2024 21:06:27 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 03 Oct 2024 17:06:27 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 3 Oct 2024 17:06:27 -0400 From: Jeff King To: Derrick Stolee Cc: git@vger.kernel.org, gitster@pobox.com, ps@pks.im Subject: [PATCH 1/5] line-log: use diff_line_prefix() instead of custom helper Message-ID: <20241003210627.GA11328@coredump.intra.peff.net> References: <20241003210548.GB11180@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241003210548.GB11180@coredump.intra.peff.net> Our local output_prefix() is exactly the same as the public diff_line_prefix() function. Let's just use that one, saving us a little bit of code. Signed-off-by: Jeff King --- line-log.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/line-log.c b/line-log.c index 29cf66bdd1..63945c4729 100644 --- a/line-log.c +++ b/line-log.c @@ -897,16 +897,6 @@ static void print_line(const char *prefix, char first, fputs("\\ No newline at end of file\n", file); } -static const char *output_prefix(struct diff_options *opt) -{ - if (opt->output_prefix) { - struct strbuf *sb = opt->output_prefix(opt, opt->output_prefix_data); - return sb->buf; - } else { - return ""; - } -} - static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *range) { unsigned int i, j = 0; @@ -916,7 +906,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang struct diff_ranges *diff = &range->diff; struct diff_options *opt = &rev->diffopt; - const char *prefix = output_prefix(opt); + const char *prefix = diff_line_prefix(opt); const char *c_reset = diff_get_color(opt->use_color, DIFF_RESET); const char *c_frag = diff_get_color(opt->use_color, DIFF_FRAGINFO); const char *c_meta = diff_get_color(opt->use_color, DIFF_METAINFO); @@ -1011,7 +1001,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang */ static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range) { - const char *prefix = output_prefix(&rev->diffopt); + const char *prefix = diff_line_prefix(&rev->diffopt); fprintf(rev->diffopt.file, "%s\n", prefix); From patchwork Thu Oct 3 21:06:54 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13821583 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D21951C9B6B for ; Thu, 3 Oct 2024 21:06:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989618; cv=none; b=VGI/Fy+6lI3v+66mlQsfJ94OcwrS7Ahz5m70wuFT3JXiM8iIt1NG2XGAdZsT5IyaDxUGT2gI1A4Dd/imxENKnaGCLgANLSa++sGC6XQCSAAhxBJXnrGfVQ+lHgCzFxICxjkagNNYqSuWOrI0C+6ePNuWq+KEkZqRkf39ET53iow= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989618; c=relaxed/simple; bh=NnbBtYAH5BKwdOQe6GZlQ3bYMNszQKleTPgArwBNXJ8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=IKYJuy23gQPNKRuqaubqv53MzoZb+Alid+FXgr613//oM3fhXCfJHwhar3S8sBMPbPpcCEDLiI+PWRfJ8sW/VeinC63RvtO5eFWtHuBrEBWWSXQ0+5s1gIx2kFBYG6TuG6BBQQW3QhKb12c/lE7n2o71XV+u983QgBNEvZgddEw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=YCpYgKd1; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="YCpYgKd1" Received: (qmail 19329 invoked by uid 109); 3 Oct 2024 21:06:56 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=NnbBtYAH5BKwdOQe6GZlQ3bYMNszQKleTPgArwBNXJ8=; b=YCpYgKd11v6A+zedaxFYr4JSCNOQh1s+cJt6JXm8TqR7MHA+/+OSyEfBl1e7saFdSJTCHLvgtIMKvKv1uz9bNpdm/07AjkbpozAO9fwJGOw6vFwfwCHbuAs7AosVFzIh8IuzS7E7paXGQuq1KnJabq2XvL9RV3AMXrkeYJCuTJ31qM++p7+qKGQb1VLXgN8NadCimDQuPFWgUot/5put2Yz6CSTUT+XKr255/WKZfBO9bYaXbKYUXJd3fpxKkNY5rQteb5RZN+/yS5zWq+hDKkp4MGsNHWKdjnadmdjxZlN7b91l4JxyeiM5147IFcUPjvyk7/X8jhPhAAPpn4d/Kg== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 03 Oct 2024 21:06:56 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 21766 invoked by uid 111); 3 Oct 2024 21:06:55 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 03 Oct 2024 17:06:55 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 3 Oct 2024 17:06:54 -0400 From: Jeff King To: Derrick Stolee Cc: git@vger.kernel.org, gitster@pobox.com, ps@pks.im Subject: [PATCH 2/5] diff: drop line_prefix_length field Message-ID: <20241003210654.GB11328@coredump.intra.peff.net> References: <20241003210548.GB11180@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241003210548.GB11180@coredump.intra.peff.net> The diff_options structure holds a line_prefix string and an associated length. But the length is always just the strlen() of the NUL-terminated string. Let's simplify the code by just storing the string pointer and assuming it is NUL-terminated when we use it. This will cause us to compute the string length in a few extra spots, but I don't think any of these are particularly hot code paths. Signed-off-by: Jeff King --- diff.c | 1 - diff.h | 1 - graph.c | 8 ++------ 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/diff.c b/diff.c index 173cbe2bed..858e001993 100644 --- a/diff.c +++ b/diff.c @@ -5400,7 +5400,6 @@ static int diff_opt_line_prefix(const struct option *opt, BUG_ON_OPT_NEG(unset); options->line_prefix = optarg; - options->line_prefix_length = strlen(options->line_prefix); graph_setup_line_prefix(options); return 0; } diff --git a/diff.h b/diff.h index 0cde3b34e2..ea3634106d 100644 --- a/diff.h +++ b/diff.h @@ -274,7 +274,6 @@ struct diff_options { const char *single_follow; const char *a_prefix, *b_prefix; const char *line_prefix; - size_t line_prefix_length; /** * collection of boolean options that affects the operation, but some do diff --git a/graph.c b/graph.c index 091c14cf4f..2cee47294f 100644 --- a/graph.c +++ b/graph.c @@ -76,10 +76,7 @@ static void graph_show_line_prefix(const struct diff_options *diffopt) if (!diffopt || !diffopt->line_prefix) return; - fwrite(diffopt->line_prefix, - sizeof(char), - diffopt->line_prefix_length, - diffopt->file); + fputs(diffopt->line_prefix, diffopt->file); } static const char **column_colors; @@ -323,8 +320,7 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void strbuf_reset(&msgbuf); if (opt->line_prefix) - strbuf_add(&msgbuf, opt->line_prefix, - opt->line_prefix_length); + strbuf_addstr(&msgbuf, opt->line_prefix); if (graph) graph_padding_line(graph, &msgbuf); return &msgbuf; From patchwork Thu Oct 3 21:09:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13821584 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A0D551AB500 for ; Thu, 3 Oct 2024 21:09:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989769; cv=none; b=CAhzkU4P4EQ3l72TobXsVDbvuB6tou+QgGxcTorRmjtrHihAb/S1pVKuLY3OOd7wRSd5jBTvprxnqbE9APwWX3Pc/GLOzEJWjkSn7hSP2RhPowp/fkIgoWvL83h8yX/6Ey3dha+31P5HtANDiAC9cQawSitXiGQp4t3N1UMb7y8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989769; c=relaxed/simple; bh=F94Fh+XJA+DYvsJ9sSxKltS9TMX7yI12PU7GW7NH/Ds=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VBeDkCCUIVmNnc56HhQERhyvNOBRc7OjWTypEwjZVzTitMzmovbH7b4AITFbZ+yhSk2JHdvsU4iRgPCz3Oqxae2V5Pj/0IHVPvcdnSQRk9cNRoXCMcEr4mTyDxFAH43uTG1FQvUeEKWoSEU5uAkGNqSKmLd0ZxqmAtROBqN9uxU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=O1Bn5Uj1; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="O1Bn5Uj1" Received: (qmail 19351 invoked by uid 109); 3 Oct 2024 21:09:26 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=F94Fh+XJA+DYvsJ9sSxKltS9TMX7yI12PU7GW7NH/Ds=; b=O1Bn5Uj13GZI3B4S2QaBP5Huuw5aMA+y9M9Gt5u47Z28n5KERJ6UpuLEpdxfQ/TGRaKVJJKw6BwFC5Nmao/uguaG9DJQLAsv4zP/hgOxJUpre8ev6gfKkqiLiro2G+aPPiJS7MBjHiXmzAGQZlx+T7TZbKBAAnnOteU8kqsVfEZ9Jsw0epFJtEIAHyBO9TQh4GnCpe02JBoYJTpV+tC+z1tTiiIjQHsJxdYiba0zfCIIEvH3OQ1J+ZKKvt8CiY6DwyiGHzBfi97Nq2hZ3/57q16BmLS0IT4ytI22p8Ul/fA2v9Xvilw2Tn3jhSNPQMCB/KEdgZLJ6hWQRRxIA/Pwlg== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 03 Oct 2024 21:09:25 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 21784 invoked by uid 111); 3 Oct 2024 21:09:25 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 03 Oct 2024 17:09:25 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 3 Oct 2024 17:09:24 -0400 From: Jeff King To: Derrick Stolee Cc: git@vger.kernel.org, gitster@pobox.com, ps@pks.im Subject: [PATCH 3/5] diff: return const char from output_prefix callback Message-ID: <20241003210924.GC11328@coredump.intra.peff.net> References: <20241003210548.GB11180@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241003210548.GB11180@coredump.intra.peff.net> 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 --- diff-lib.c | 4 ++-- diff.c | 9 +++------ diff.h | 2 +- graph.c | 4 ++-- log-tree.c | 7 +------ range-diff.c | 4 ++-- 6 files changed, 11 insertions(+), 19 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index a680768ee7..6b14b95962 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 858e001993..6c96154fed 100644 --- a/diff.c +++ b/diff.c @@ -2317,12 +2317,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 ea3634106d..5c8de79535 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 2cee47294f..c046f6285d 100644 --- a/graph.c +++ b/graph.c @@ -311,7 +311,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; @@ -323,7 +323,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 3758e0d3b8..33eb96b50c 100644 --- 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 bbb0952264..10885ba301 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); /* From patchwork Thu Oct 3 21:11:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13821586 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1519F1DFFC for ; Thu, 3 Oct 2024 21:11:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989894; cv=none; b=nb98EAwgc0DzQ7n/y72Hx7XDsJc3Ee5gVNpGVu2EsNoh5nIUNtg8ic9AvXgJX5uEI4cAGbfldHVgqr8ItAOnURsQbyAh2Y2DtuC77AMrgfWngVosVAa3NX/TYDIi5RxB/IoCqD9lJDmWk4BaNUARw5NCtol6Ji0rR5Rpv2iNR/E= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727989894; c=relaxed/simple; bh=6y7U4n4G/286rRqLaMCZJF3x5vhICq0YgRlwE7WzHQo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=B996HXZNZgNhTFFoTcg7u7jmh0mlHTDQ0PU5Qve6HL1fpV4faJlMkFDmPUmFruQvpwiypdydybBYgxP5qOvpddEQvWERTuQP95VMuNuzecLreTgAQ0t7R66OCIPuK/3vFkm1FJzIHZgCeXY6f29xfGYtaNBWIMSYTa1r187STWM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=XcqKwUPZ; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="XcqKwUPZ" Received: (qmail 19368 invoked by uid 109); 3 Oct 2024 21:11:32 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=6y7U4n4G/286rRqLaMCZJF3x5vhICq0YgRlwE7WzHQo=; b=XcqKwUPZYf1wXC8zyC96F/8hbJzS8nT+xaMJW1SOaBgOD4COVFU88V2nnl0pVvkOtJe9pCNPViaLLO43tEjzMM78KmwxKJI0B2CdxnvpHRp0ldic1X6bng7ph1xh27ARHtCwRidbE0VgqDCfvbLkOzj3UbaC+snxp0Z5+w8BRHnmiJv6cxLpu6deHkRV52AetHDuEqhiIfN949s4ibVLbvd8zi7yWuwJHHkSbVhkbu7eD0kX6Kwd9kBNSKpstgPXKWa4VdKeSDm8wyr7aUayE63/S8uVFHFIwwsby8ich44Hxmgync+60alpKARqjEenb7pv07dETUIhT53+ZrSCEQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 03 Oct 2024 21:11:32 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 21827 invoked by uid 111); 3 Oct 2024 21:11:31 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 03 Oct 2024 17:11:31 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 3 Oct 2024 17:11:31 -0400 From: Jeff King To: Derrick Stolee Cc: git@vger.kernel.org, gitster@pobox.com, ps@pks.im Subject: [PATCH 4/5] diff: return line_prefix directly when possible Message-ID: <20241003211131.GD11328@coredump.intra.peff.net> References: <20241003210548.GB11180@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241003210548.GB11180@coredump.intra.peff.net> We may point our output_prefix callback to diff_output_prefix_callback() in any of these cases: 1. we have a user-provided line_prefix 2. we have a graph prefix to show 3. both (1) and (2) The function combines the available elements into a strbuf and returns its pointer. In the case that we just have the line_prefix, though, there is no need for the strbuf. We can return the string directly. This is a minor optimization by itself, but also will allow us to clean up some memory ownership issues on top. Signed-off-by: Jeff King --- The old code did handle a case "0" where neither is set. But in that case we would never set up the callback in the first place. So I didn't think it was worth being defensive there. graph.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/graph.c b/graph.c index c046f6285d..0d200ca77f 100644 --- a/graph.c +++ b/graph.c @@ -318,6 +318,9 @@ static const char *diff_output_prefix_callback(struct diff_options *opt, void *d assert(opt); + if (!graph) + return opt->line_prefix; + strbuf_reset(&msgbuf); if (opt->line_prefix) strbuf_addstr(&msgbuf, opt->line_prefix); From patchwork Thu Oct 3 21:13:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13821587 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6D9071AC423 for ; Thu, 3 Oct 2024 21:13:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727990001; cv=none; b=h3eMZCcM99SOoSUGlionT+Fv3FRq3lolzXitkAohL2/Lt1/RaY8HNfn5BbuSR3jeRM2iTRH8qHiDEY0fcq4Rja6GCMppruUonhiZHzwO80eiEroeALuj2ly0adUJa3xyuJ1U3oDArsWzpeU3VmXW9zJiEhp9/q/Op7QVf/0z5/I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727990001; c=relaxed/simple; bh=utSH1cWBnMfyl7ozS7L9Q9KEEYG0lWRKBlr5c4elyrM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=u5MgqRuAVY4pL5oeBuN/ka/turI5nPeZA0d1fG4j64z2dF7TQVfItqziWMUXih59qEJG9KHSk8YiWm0+DuAKFMECNV/vDA1DuwLm7T4L73l4x1qbrJgoxUp+P29t7J6hmg4LRhlqQo5OqHZTkILZAqw4LFF1cIJZ3moIp2sVdj0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=MLhuVDDr; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="MLhuVDDr" Received: (qmail 19382 invoked by uid 109); 3 Oct 2024 21:13:18 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=utSH1cWBnMfyl7ozS7L9Q9KEEYG0lWRKBlr5c4elyrM=; b=MLhuVDDrjeg/ISL6StARpsfzaKRGK0zM4nxOk620DJvvr/XRAmxzDhLu3IV/SHYeaDcWU8JILVF13GbnsO798S0a2O450oVxQJghEP8TkC9M0gtqkWzuiMaWXH553krXdee0dfyPSykXFjNzS7ULK+xrb+zJJU1jciYrs7nSJucvSuiGCfqiYFz1Yj3PHWXRtfCTkouw4WjidzXGEidBlkPYlPskvBZH9EQ1lB2+qHau8yATT/DccpxjiCDUEu0K+HGRkOw/sKiOyhyydBbsM1Bzf6OEEVXbsM6ktLMTD1sEkClBXLB9Rb+IdtL/iqwvLyFN3fP2JG8sT2Il09JmiQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 03 Oct 2024 21:13:18 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 21835 invoked by uid 111); 3 Oct 2024 21:13:18 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 03 Oct 2024 17:13:18 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 3 Oct 2024 17:13:17 -0400 From: Jeff King To: Derrick Stolee Cc: git@vger.kernel.org, gitster@pobox.com, ps@pks.im Subject: [PATCH 5/5] diff: store graph prefix buf in git_graph struct Message-ID: <20241003211317.GE11328@coredump.intra.peff.net> References: <20241003210548.GB11180@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241003210548.GB11180@coredump.intra.peff.net> The diffopt output_prefix interface makes it the callback's job to handle ownership of the memory it returns, keeping it valid while callers are using it and then eventually freeing it when we are done diffing. In diff_output_prefix_callback() we handle this with a static strbuf, effectively "leaking" it when the diff is done (but not triggering any leak detectors because it's technically still reachable). This has not been a big problem in practice, but it is a problem for libification: two diffs running in the same process could stomp on each other's prefix buffers. Since we only need the strbuf when we are formatting graph padding, we can give ownership of the strbuf to the git_graph struct, letting us free it when that struct is no longer in use. Signed-off-by: Jeff King --- graph.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/graph.c b/graph.c index 0d200ca77f..bf000fdbe1 100644 --- a/graph.c +++ b/graph.c @@ -309,24 +309,28 @@ struct git_graph { * stored as an index into the array column_colors. */ unsigned short default_column_color; + + /* + * Scratch buffer for generating prefixes to be used with + * diff_output_prefix_callback(). + */ + struct strbuf prefix_buf; }; static const char *diff_output_prefix_callback(struct diff_options *opt, void *data) { struct git_graph *graph = data; - static struct strbuf msgbuf = STRBUF_INIT; assert(opt); if (!graph) return opt->line_prefix; - strbuf_reset(&msgbuf); + strbuf_reset(&graph->prefix_buf); if (opt->line_prefix) - strbuf_addstr(&msgbuf, opt->line_prefix); - if (graph) - graph_padding_line(graph, &msgbuf); - return msgbuf.buf; + strbuf_addstr(&graph->prefix_buf, opt->line_prefix); + graph_padding_line(graph, &graph->prefix_buf); + return graph->prefix_buf.buf; } static const struct diff_options *default_diffopt; @@ -396,6 +400,7 @@ struct git_graph *graph_init(struct rev_info *opt) * The diff output prefix callback, with this we can make * all the diff output to align with the graph lines. */ + strbuf_init(&graph->prefix_buf, 0); opt->diffopt.output_prefix = diff_output_prefix_callback; opt->diffopt.output_prefix_data = graph; @@ -411,6 +416,7 @@ void graph_clear(struct git_graph *graph) free(graph->new_columns); free(graph->mapping); free(graph->old_mapping); + strbuf_release(&graph->prefix_buf); free(graph); }