sequencer.c: check for lock failure and bail early in fast_forward_to

Change fast_forward_to() to check if locking the ref failed, print a nice error message and bail out early. The old code did not check if ref_lock was NULL and relied on the fact that the write_ref_sha1() would safely detect this condition and set the return variable ret to indicate an error. While that is safe, it makes the code harder to read for two reasons: * Inconsistency. Almost all other places we do check the lock for NULL explicitly, so the naive reader is confused "why don't we check here?" * And relying on write_ref_sha1() to detect and return an error for when a previous lock_any_ref_for_update() failed feels obfuscated. This change should not change any functionality or logic aside from adding an extra error message when this condition is triggered (write_ref_sha1() returns an error silently for this condition). Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ronnie Sahlberg committed Apr 16, 2014 at 11:56 UTC 651ab9f553a1fd8bb847bd42922dacb14f8ec77f
1 file changed +4
sequencer.c
+4
@@ -281,8 +281,12 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
281 exit(1); /* the callee should have complained already */
282 ref_lock = lock_any_ref_for_update("HEAD", unborn ? null_sha1 : from,
283 0, NULL);
284 + if (!ref_lock)
285 + return error(_("Failed to lock HEAD during fast_forward_to"));
286 +
287 strbuf_addf(&sb, "%s: fast-forward", action_name(opts));
288 ret = write_ref_sha1(ref_lock, to, sb.buf);
289 +
290 strbuf_release(&sb);
291 return ret;
292 }