From 6e30b2f652d0a6748e2041dee5b5612cafca29b2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 25 Apr 2016 17:48:32 +0200 Subject: lock_ref_for_update(): don't resolve symrefs If a transaction includes a non-NODEREF update to a symbolic reference, we don't have to look it up in lock_ref_for_update(). The reference will be dereferenced anyway when the split-off update is processed. This change requires that we store a backpointer from the split-off update to its parent update, for two reasons: * We still want to report the original reference name in error messages. So if an error occurs when checking the split-off update's old_sha1, walk the parent_update pointers back to find the original reference name, and report that one. * We still need to write the old_sha1 of the symref to its reflog. So after we read the split-off update's reference value, walk the parent_update pointers back and fill in their old_sha1 fields. Aside from eliminating unnecessary reads, this change fixes a subtle (though not very serious) race condition: in the old code, the old_sha1 of the symref was resolved before the reference that it pointed at was locked. So it was possible that the old_sha1 value logged to the symref's reflog could be wrong if another process changed the downstream reference before it was locked. Signed-off-by: Michael Haggerty --- refs/refs-internal.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'refs/refs-internal.h') diff --git a/refs/refs-internal.h b/refs/refs-internal.h index cccd76b28c..1bb3d87dc5 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -143,24 +143,41 @@ int should_autocreate_reflog(const char *refname); * not exist before update. */ struct ref_update { + /* * If (flags & REF_HAVE_NEW), set the reference to this value: */ unsigned char new_sha1[20]; + /* * If (flags & REF_HAVE_OLD), check that the reference * previously had this value: */ unsigned char old_sha1[20]; + /* * One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF, * REF_DELETING, REF_ISPRUNING, REF_LOG_ONLY, and * REF_UPDATE_VIA_HEAD: */ unsigned int flags; + struct ref_lock *lock; unsigned int type; char *msg; + + /* + * If this ref_update was split off of a symref update via + * split_symref_update(), then this member points at that + * update. This is used for two purposes: + * 1. When reporting errors, we report the refname under which + * the update was originally requested. + * 2. When we read the old value of this reference, we + * propagate it back to its parent update for recording in + * the latter's reflog. + */ + struct ref_update *parent_update; + const char refname[FLEX_ARRAY]; }; -- cgit v1.3-5-g9baa