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

From: Jeff King <peff@peff.net>
Date: Sep 27, 2024, 03:55 UTC
Message-ID: <20240927035508.GF567395@coredump.intra.peff.net>
In-Reply-To: <ZvVmoARjOAiAqiIz@pks.im>

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

Show 16 quoted lines

> > 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