Subject: Re: [PATCH 20/28] http-push: free repo->url string

From: Jeff King <[email-masked]>
Date: Thu, 26 Sep 2024 23:55:08 -0400

On Thu, Sep 26, 2024 at 03:50:24PM +0200, Patrick Steinhardt wrote:

> > diff --git a/http-push.c b/http-push.c
> > index f60b2ceba5..52c53928a9 100644
> > --- a/http-push.c
> > +++ b/http-push.c
> > @@ -1972,6 +1972,7 @@ int cmd_main(int argc, const char **argv)
> >   cleanup:
> >  	if (info_ref_lock)
> >  		unlock_remote(info_ref_lock);
> > +	free(repo->url);
> >  	free(repo);
> >  
> >  	http_cleanup();
> 
> I was wondering whether we also need to free `repo->path`, which is a
> `char *` as well. But that is only being assigned pointers into command
> line argument strings, and thus we do not have to free it.

Yeah, I think that "path" has the wrong type, and should be "const char
*". I was a little surprised the compiler does not complain, but it is
the old C gotcha: we assign to it via strchr(), which launders away the
const.

It would probably be OK to fix, though in general my dream is still that
we'd delete all of this code in the not too distant future. Even if we
keep dumb-http fetch for its resumability, dumb-http push just seems
like a useless holdover from the early days.

-Peff