[0/28] leak fixes for http fetch/push

Message

Jeff King Sept. 24, 2024, 9:49 p.m. UTC

Patrick asked me to take a look at the remaining leaks in the http
push/fetch code, especially the dumb variants. So here are enough
patches to all of these scripts running leak-free:

  t5500-fetch-pack.sh
  t5539-fetch-http-shallow.sh
  t5540-http-push-webdav.sh
  t5541-http-push-smart.sh
  t5542-push-http-shallow.sh
  t5550-http-fetch-dumb.sh
  t5551-http-fetch-smart.sh
  t5582-fetch-negative-refspec.sh
  t5619-clone-local-ambiguous-transport.sh

I did have to cheat a little and steal some of the patches from his
"leak fixes part 8" work-in-progress (which are at the front of this
series). But otherwise this should be independent of the other
leak-fixing work and can be applied directly on master.

I tried to put patches touching similar areas together, but there's
probably still a bit of a jumble, just because I got there by picking at
the pile of leak logs for each script. :)

Most of them are pretty obvious "add a free call" one-liners. If you
really want to put on your thinking cap, try patches 7, 14, and 21 (I'm
going to pretend I spaced them out evenly for your reading pleasure).

  [01/28]: http-fetch: clear leaking git-index-pack(1) arguments
  [02/28]: shallow: fix leak when unregistering last shallow root
  [03/28]: fetch-pack: fix leaking sought refs
  [04/28]: connect: clear child process before freeing in diagnostic mode
  [05/28]: fetch-pack: free object filter before exiting
  [06/28]: fetch-pack, send-pack: clean up shallow oid array
  [07/28]: commit: avoid leaking already-saved buffer
  [08/28]: send-pack: free cas options before exit
  [09/28]: transport-helper: fix strbuf leak in push_refs_with_push()
  [10/28]: fetch: free "raw" string when shrinking refspec
  [11/28]: fetch-pack: clear pack lockfiles list
  [12/28]: transport-helper: fix leak of dummy refs_list
  [13/28]: http: fix leak when redacting cookies from curl trace
  [14/28]: http: fix leak of http_object_request struct
  [15/28]: http: call git_inflate_end() when releasing http_object_request
  [16/28]: http: stop leaking buffer in http_get_info_packs()
  [17/28]: remote-curl: free HEAD ref with free_one_ref()
  [18/28]: http-walker: free fake packed_git list
  [19/28]: http-push: clear refspecs before exiting
  [20/28]: http-push: free repo->url string
  [21/28]: http-push: free curl header lists
  [22/28]: http-push: free transfer_request dest field
  [23/28]: http-push: free transfer_request strbuf
  [24/28]: http-push: free remote_ls_ctx.dentry_name
  [25/28]: http-push: free xml_ctx.cdata after use
  [26/28]: http-push: clean up objects list
  [27/28]: http-push: clean up loose request when falling back to packed
  [28/28]: http-push: clean up local_refs at exit

 builtin/fetch-pack.c                       | 14 +++++++-
 builtin/fetch.c                            |  1 +
 builtin/push.c                             |  1 +
 builtin/send-pack.c                        |  2 ++
 commit.c                                   |  3 +-
 connect.c                                  |  1 +
 http-fetch.c                               | 16 ++++++---
 http-push.c                                | 40 +++++++++++++++-------
 http-walker.c                              | 18 +++++++---
 http.c                                     | 16 ++++++---
 http.h                                     |  4 +--
 refspec.c                                  |  2 +-
 refspec.h                                  |  2 +-
 remote-curl.c                              |  2 +-
 remote.c                                   |  2 +-
 remote.h                                   |  1 +
 shallow.c                                  |  5 ++-
 t/t5500-fetch-pack.sh                      |  1 +
 t/t5539-fetch-http-shallow.sh              |  1 +
 t/t5540-http-push-webdav.sh                |  1 +
 t/t5541-http-push-smart.sh                 |  1 +
 t/t5542-push-http-shallow.sh               |  1 +
 t/t5550-http-fetch-dumb.sh                 |  1 +
 t/t5551-http-fetch-smart.sh                |  1 +
 t/t5582-fetch-negative-refspec.sh          |  1 +
 t/t5619-clone-local-ambiguous-transport.sh |  1 +
 t/t5700-protocol-v1.sh                     |  1 +
 transport-helper.c                         | 11 ++++--
 transport.c                                | 11 +++++-
 29 files changed, 123 insertions(+), 39 deletions(-)

-Peff

Comments

Patrick Steinhardt Sept. 26, 2024, 1:50 p.m. UTC | #1

On Tue, Sep 24, 2024 at 05:49:30PM -0400, Jeff King wrote:
> Patrick asked me to take a look at the remaining leaks in the http
> push/fetch code, especially the dumb variants. So here are enough
> patches to all of these scripts running leak-free:

Thank you for taking a look at this, highly appreciated! The series
looks good to me, I've only got a couple of nits regarding typos that
you may or may not want to address. Either way is fine with me.

Patrick

Jeff King Sept. 27, 2024, 3:55 a.m. UTC | #2

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

> On Tue, Sep 24, 2024 at 05:49:30PM -0400, Jeff King wrote:
> > Patrick asked me to take a look at the remaining leaks in the http
> > push/fetch code, especially the dumb variants. So here are enough
> > patches to all of these scripts running leak-free:
> 
> Thank you for taking a look at this, highly appreciated! The series
> looks good to me, I've only got a couple of nits regarding typos that
> you may or may not want to address. Either way is fine with me.

I responded to a few, but I don't think any of them really merits a
re-roll. Thanks for reviewing!

-Peff