[PATCH v2 17/25] commit_lock_file(): make committing an unlocked lockfile a NOP

[Michael Haggerty <mhagger@alum.mit.edu>]

It was previously a bug to call commit_lock_file() with a lock_file
object that was not active (an illegal access would happen within the
function).  It was presumably never done, but this would be an easy
programming error to overlook.  So guard the file-renaming code with
an if statement to change committing an unlocked file into a NOP.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Alternatively, we could make it a fatal error (e.g., an assert() or
if...die()) to call commit_lock_file() on an unlocked file, or omit a
warning in this case.  But since it is so hard to test code related to
locking failures, I have the feeling that such an error is most likely
to occur in some error-handling path, maybe when some other lockfile
acquisition has failed, and it would be better to let the code
continue its attempted cleanup instead of dying.  But it would be easy
to persuade me to change my opinion.

 Documentation/technical/api-lockfile.txt | 4 +++-
 lockfile.c                               | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt
index b53e300..bfb2676 100644
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -68,7 +68,9 @@ commit_lock_file::
 	with an earlier call to `hold_lock_file_for_update()`,
 	close the file descriptor and rename the lockfile to its
 	final destination.  Returns 0 upon success, a negative
-	value on failure to close(2) or rename(2).
+	value on failure to close(2) or rename(2).  It is a NOP to
+	call `commit_lock_file()` for a `lock_file` object that is not
+	currently locked.
 
 rollback_lock_file::
 
diff --git a/lockfile.c b/lockfile.c
index 47762c6..24aaa53 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -301,6 +301,9 @@ int commit_lock_file(struct lock_file *lk)
 	if (lk->fd >= 0 && close_lock_file(lk))
 		return -1;
 
+	if (!lk->filename[0])
+		return 0;
+
 	strcpy(result_file, lk->filename);
 	/* remove ".lock": */
 	result_file[strlen(result_file) - LOCK_SUFFIX_LEN] = 0;
-- 
1.9.1