Re: [PATCH] run-command: avoid undefined behavior in exists_in_PATH

> brian m. carlson wrote:
> 
> > In this function, we free the pointer we get from locate_in_PATH and
> > then check whether it's NULL.  However, this is undefined behavior if
> > the pointer is non-NULL, since the C standard no longer permits us to
> > use a valid pointer after freeing it.
> [...]
> > Noticed-by: Miriam R. <mirucam@gmail.com>
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  run-command.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> 
> This API that forces the caller to deal with the allocated result when
> they never asked for it seems a bit inconvenient.  Should we clean it up
> a little?  Something like this (on top):
> 
> -- >8 --
> Subject: run-command: let caller pass in buffer to locate_in_PATH
> 
> Instead of returning a buffer that the caller is responsible for
> freeing, use a strbuf output parameter to record the path to the
> searched-for program.
> 
> This makes ownership a little easier to reason about, since the owning
> code declares the buffer.  It's a good habit to follow because it
> allows buffer reuse when calling such a function in a loop.
> 
> It also allows the caller exists_in_PATH that does not care about the
> path to the command to be slightly simplified, by allowing a NULL
> output parameter that means that locate_in_PATH should take care of
> allocating and freeing its temporary buffer.