Subject: Re: [PATCH v2 00/33] Yet more preparation for reference backends

From: Michael Haggerty <[email-masked]>
Date: Fri, 10 Jun 2016 14:50:31 +0200

On 05/06/2016 06:13 PM, Michael Haggerty wrote:
> [...]
> This patch series is also available from my GitHub repo [2] as branch
> "split-under-lock".
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/292772
> [2] https://github.com/mhagger/git

I was reading this area of the code again, and I found a problem in this
patch series. Now that ref_updates can be added to ref_transactions
while they are being processed, it is not correct to store a pointer to
transaction->updates in ref_transaction_commit() here [1], because the
array might be moved by realloc() if it grows during the function. The
problem wasn't detected during testing because an added commit would
have to cross an ALLOC_GROW boundary to trigger the bug.

The fix is obvious but it is textually quite a few lines. For good
measure, the same fix should be made in initial_ref_transaction_commit()
here [2].

The most logical place to fix this is by expanding commit
"ref_transaction_commit(): remove local variable n" [3], so I've done
that and force pushed the result to my GitHub account [4] as branch
"split-under-lock".

Junio, if you want to incorporate this revised version of the branch in
your big rewind of next, then we can pretend that the bug was never
there :-) Otherwise, tell me in what form you would like the fix and I
will be happy to provide it.

Sorry for finding this problem so late in the process.

Michael

[1]
https://github.com/mhagger/git/blob/088c8f756c86581ff25371983ef409044b348bb9/refs/files-backend.c#L3559
[2]
https://github.com/mhagger/git/blob/088c8f756c86581ff25371983ef409044b348bb9/refs/files-backend.c#L3725
[3] http://article.gmane.org/gmane.comp.version-control.git/293801
[4] https://github.com/mhagger/git