Re: [PATCH v2 3/3] diff: modify output_prefix function pointer
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 3, 2024, 16:26 UTC
- Message-ID
- <xmqqsetdi24w.fsf@gitster.g>
- In-Reply-To
- <e1d825ad212d91505eee9d911abbd3ba6bc170b1.1727956724.git.gitgitgadget@gmail.com>
"Jeff King via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 21 quoted lines
> 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(-)
Very nice.
Show 8 quoted lines
> 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);
> }OK. We are not relying on the strbuf being able to have embedded NUL in the buffer, and this looks very sensible.
Thanks.