On Sat, Apr 06, 2024 at 10:05:00PM -0400, Eric Sunshine wrote: > On Sat, Apr 6, 2024 at 9:02 PM Jeff King <peff@xxxxxxxx> wrote: > > config: use git_config_string() in easy cases > > Did you instead mean? > > config: use git_config_string_dup() in easy cases Oops, yes. > > (like the one above), we can easily convert them to use a variant of the > > helper that frees the existing value before writing over it. That works > > because free(NULL) is a noop. What we can't change, though, is cases > > where the pointer is initialized to a string literal, like: > > Fun historical (historic?) fact: On NeXTSTEP operating system, > free(NULL) and realloc(NULL,n) would crash the program, so getting > non-native (open-source) code to run correctly often involved more > than a little effort.. I'm happy that's no longer something we need to worry about. ;) IIRC we had some discussion long ago about whether free(NULL) needed to be protected, and came to the conclusion that it's OK everywhere. (There's a discussion in a nearby thread about a possible git_free() wrapper. The point there is errno handling, but of course it could also cover this case for us if we found a modern platform where it was a problem). -Peff