Re: [PATCH v2 3/3] diff: modify output_prefix function pointer
From: Junio C Hamano <hidden>
Date: 2024-10-03 16:26:26
"Jeff King via GitGitGadget" [off-list ref] writes:
From: Jeff King <redacted> 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 <redacted> Signed-off-by: Derrick Stolee <redacted> --- 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.
quoted hunk
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.