From: Junio C Hamano Date: Tue, 24 Feb 2026 06:25:07 GMT Subject: Re: [PATCH 1/3] pretty.c: fix null pointer dereference Message-ID: In-Reply-To: <20260224040400.751247-2-mroik@delayed.space> Mirko Faina writes: > commit_format_is_empty() is used to check whether "user_format" is set > to a value. Unfortunately this function crashes the program if no > user_format is set. This is because instead of checking for the pointer > value it checks for its dereferenced value, this being NULL if > user_format is not set. > > Teach the proper condition to check if user_format is set. > > Signed-off-by: Mirko Faina > --- > pretty.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Interesting. Are any of the existing calls to this function that passes CMIT_FMT_USERFORMAT trigger a segfault with certain condition? For example, there are only two places where CMIT_FMT_USERFORMAT is assigned to something. One is save_user_format() where user_format gets a non NULL string before rev->commit_format gets assigned CMIT_FMT_USERFORMAT. Another is git_pretty_formats_config() that parses configuration variables "pretty.*" and populate commit_formats map. This is later used in get_commit_format() and that function always calls save_user_format() we just saw when the format used is CMIT_FMT_USERFORMAT. So the existing code paths seem to be safe by design. What I am wondering is if a NULL user_format should be flagged as a programming error, instead of getting swept under the rug like this patch does. IOW, int commit_format_is_empty(enum cmit_fmt fmt) { if (fmt != CMIT_FMT_USERFORMAT) return 0; if (!user_format) BUG("never called save_user_format() and using USERFORMAT?"); return !*user_format; } > > diff --git a/pretty.c b/pretty.c > index e0646bbc5d..cdb8bf559d 100644 > --- a/pretty.c > +++ b/pretty.c > @@ -47,7 +47,7 @@ static struct cmt_fmt_map *find_commit_format(const char *sought); > > int commit_format_is_empty(enum cmit_fmt fmt) > { > - return fmt == CMIT_FMT_USERFORMAT && !*user_format; > + return fmt == CMIT_FMT_USERFORMAT && !user_format; > } > > static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)