Re: [PATCH v11 26/41] walker.c: use ref transaction for ref updates

Ronnie Sahlberg wrote:

Switch to using ref transactions in walker_fetch(). As part of the refactoring
to use ref transactions we also fix a potential memory leak where in the
original code if write_ref_sha1() would fail we would end up returning from
the function without free()ing the msg string.

Sounds good.

This changes the locking slightly for walker_fetch. Previously the code would
lock all refs before writing them but now we do not lock the refs until the
commit stage.

I don't think this actually changes locking in any significant way.
(Timing changes, but that's to be expected whenever code changes.)

The old contract was:

You run git http-fetch with refnames passed with "-w".  http-fetch
will overwrite the refs with their new values.

The new contract is:

You run git http-fetch with refnames passed with "-w".  http-fetch
will overwrite the refs with their new values.

What changed?

[...]

Note that this function is only called when fetching from a remote HTTP
repository onto the local (most of the time single-user) repository which
likely means that the type of collissions that the previous locking would
protect against and cause the fetch to fail for to be even more rare.

More to the point, while this function is used by 'git remote-http' (and
hence by "git fetch https://foo/bar/baz"), it is only used to fetch
objects by that code path.  The remote helper machinery handles the ref
updates on its own (and passes NULL as the write_ref argument to the
dumb walker).

[...]

Signed-off-by: Ronnie Sahlberg <redacted>
---
 walker.c | 56 +++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 23 deletions(-)

The code change looks good from a quick glance.

gcc 4.6.3 doesn't seem to be smart enough to notice that 'transaction'
is always initialized before it is used, so it (optionally) could be
worth initializing 'transition = NULL' to work around that.

So I think this just needs a simpler commit message and then it would be
good to go.

Jonathan