git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 16/25] commit_lock_file(): die() if called for unlocked lockfile object

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Apr 14, 2014, 13:54 UTC
Message-ID
<1397483695-10888-17-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1397483695-10888-1-git-send-email-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 before continuing, do a consistency check that the lock_file object really is locked.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 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..9a94ead 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 bug 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 664b0c3..1453a7a 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -292,6 +292,9 @@ int commit_lock_file(struct lock_file *lk)
 	if (lk->fd >= 0 && close_lock_file(lk))
 		return -1;
 
+	if (!lk->filename[0])
+		die("BUG: attempt to commit unlocked object");
+
 	strcpy(result_file, lk->filename);
 	/* remove ".lock": */
 	result_file[strlen(result_file) - LOCK_SUFFIX_LEN] = 0;
-- 
1.9.1
Previous: Michael HaggertyNext: Johannes Sixt
Message 17 of 32 in “Lockfile correctness and refactoring”
  1. 00/25 Lockfile correctness and refactoringMichael Haggerty, Apr 14, 2014
  2. 01/25 unable_to_lock_die(): rename function from unable_to_lock_index_die()Michael Haggerty, Apr 14, 2014
  3. 02/25 api-lockfile: expand the documentationMichael Haggerty, Apr 14, 2014
  4. 03/25 rollback_lock_file(): do not clear filename redundantlyMichael Haggerty, Apr 14, 2014
  5. 04/25 rollback_lock_file(): set fd to -1Michael Haggerty, Apr 14, 2014
  6. 05/25 lockfile: unlock file if lockfile permissions cannot be adjustedMichael Haggerty, Apr 14, 2014
  7. 06/25 hold_lock_file_for_append(): release lock on errorsMichael Haggerty, Apr 14, 2014
  8. 07/25 lock_file(): always add lock_file object to lock_file_listMichael Haggerty, Apr 14, 2014
  9. 08/25 lockfile.c: document the various states of lock_file objectsMichael Haggerty, Apr 14, 2014
  10. 09/25 cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LENMichael Haggerty, Apr 14, 2014
  11. 10/25 delete_ref_loose(): don't muck around in the lock_file's filenameMichael Haggerty, Apr 14, 2014
  12. 11/25 prepare_index(): declare return value to be (const char *)Michael Haggerty, Apr 14, 2014
  13. 12/25 write_packed_entry_fn(): convert cb_data into a (const int *)Michael Haggerty, Apr 14, 2014
  14. 13/25 lock_file(): exit early if lockfile cannot be openedMichael Haggerty, Apr 14, 2014
  15. 14/25 remove_lock_file(): call rollback_lock_file()Michael Haggerty, Apr 14, 2014
  16. 15/25 commit_lock_file(): inline temporary variableMichael Haggerty, Apr 14, 2014
  17. 16/25 commit_lock_file(): die() if called for unlocked lockfile objectMichael Haggerty, Apr 14, 2014
  18. Johannes SixtApr 15, 2014
  19. Michael HaggertyApr 16, 2014
  20. 17/25 lockfile: avoid transitory invalid statesMichael Haggerty, Apr 14, 2014
  21. 18/25 struct lock_file: declare some fields volatileMichael Haggerty, Apr 14, 2014
  22. Johannes SixtApr 15, 2014
  23. Michael HaggertyApr 16, 2014
  24. 19/25 try_merge_strategy(): remove redundant lock_file allocationMichael Haggerty, Apr 14, 2014
  25. 20/25 try_merge_strategy(): use a statically-allocated lock_file objectMichael Haggerty, Apr 14, 2014
  26. 21/25 commit_lock_file(): use a strbuf to manage temporary spaceMichael Haggerty, Apr 14, 2014
  27. 22/25 Change lock_file::filename into a strbufMichael Haggerty, Apr 14, 2014
  28. 23/25 resolve_symlink(): use a strbuf for internal scratch spaceMichael Haggerty, Apr 14, 2014
  29. 24/25 resolve_symlink(): take a strbuf parameterMichael Haggerty, Apr 14, 2014
  30. 25/25 trim_last_path_elm(): replace last_path_elm()Michael Haggerty, Apr 14, 2014
  31. Torsten BögershausenApr 15, 2014
  32. Michael HaggertyApr 16, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.