On Tue, Aug 20, 2024 at 01:36:11PM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> writes: > > > When parsing pretty formats from the config we leak the name and user > > format whenever these are set multiple times. This is because we do not > > free any already-set value in case there is one. > > > > Plugging this leak for the name is trivial. For the user format we need > > to be a bit more careful, because we may end up assigning a pointer into > > the allocated region when the string is prefixed with either "format" or > > "tformat:". In order to make it safe to unconditionally free the user > > format we thus strdup the stripped string into the field instead of a > > pointer into the string. > > Yup, the stripped one is trickier, but the change looks correct. > > Will queue. > > By the way, we tend to prefer no spaces after (cast) in our > codebase, but I just noticed that it is not spelled out in the > coding guidelines. Comparing > > $ git grep -E -e '\((int|char \*)\) ' \*.c ':!compat/' ':!contrib/' > $ git grep -E -e '\((int|char \*)\)[^ ]' \*.c ':!compat/' ':!contrib/' > > tells me that the extra space after the (cast) is found mostly in > borrowed or imported sources and majority of culprits are found in > reftable library X-<. Not entirely surprising. I myself have typically favored adding a space after the cast, and I remember wondering about the coding style several times here. I never wondered enough to actually check though. So thanks for clarifying and thanks for updating the coding guidelines around this. Patrick
