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

[PATCH v2 24/25] resolve_symlink(): take a strbuf parameter

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Apr 6, 2014, 23:34 UTC
Message-ID
<1396827247-28465-25-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1396827247-28465-1-git-send-email-mhagger@alum.mit.edu>

Change resolve_symlink() to take a strbuf rather than a string as parameter. This simplifies the code and removes an arbitrary pathname length restriction. It also means that lock_file's filename field no longer needs to be initialized to a large size.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 lockfile.c | 53 +++++++++++++++++++----------------------------------
 1 file changed, 19 insertions(+), 34 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index c4e32a9..8e7bcda 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -133,53 +133,40 @@ static char *last_path_elm(char *p)
 #define MAXDEPTH 5
 
 /*
- * p = path that may be a symlink
- * s = full size of p
+ * path contains a path that might be a symlink.
  *
- * If p is a symlink, attempt to overwrite p with a path to the real
- * file or directory (which may or may not exist), following a chain of
- * symlinks if necessary.  Otherwise, leave p unmodified.
+ * If path is a symlink, attempt to overwrite it with a path to the
+ * real file or directory (which may or may not exist), following a
+ * chain of symlinks if necessary.  Otherwise, leave path unmodified.
  *
- * This is a best-effort routine.  If an error occurs, p will either be
- * left unmodified or will name a different symlink in a symlink chain
- * that started with p's initial contents.
- *
- * Always returns p.
+ * This is a best-effort routine.  If an error occurs, path will
+ * either be left unmodified or will name a different symlink in a
+ * symlink chain that started with the original path.
  */
-
-static char *resolve_symlink(char *p, size_t s)
+static void resolve_symlink(struct strbuf *path)
 {
 	int depth = MAXDEPTH;
 	static struct strbuf link = STRBUF_INIT;
 
 	while (depth--) {
-		if (strbuf_readlink(&link, p, strlen(p)) < 0)
+		if (strbuf_readlink(&link, path->buf, path->len) < 0)
 			break;
 
-		if (is_absolute_path(link.buf)) {
+		if (is_absolute_path(link.buf))
 			/* absolute path simply replaces p */
-			if (link.len < s)
-				strcpy(p, link.buf);
-			else {
-				warning("%s: symlink too long", p);
-				break;
-			}
-		} else {
+			strbuf_reset(path);
+		else {
 			/*
 			 * link is a relative path, so replace the
 			 * last element of p with it.
 			 */
-			char *r = (char *)last_path_elm(p);
-			if (r - p + link.len < s)
-				strcpy(r, link.buf);
-			else {
-				warning("%s: symlink too long", p);
-				break;
-			}
+			char *r = last_path_elm(path->buf);
+			strbuf_setlen(path, r - path->buf);
 		}
+
+		strbuf_addbuf(path, &link);
 	}
 	strbuf_reset(&link);
-	return p;
 }
 
 /* We append ".lock" to the filename to derive the lockfile name: */
@@ -200,16 +187,14 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 		lk->fd = -1;
 		lk->owner = 0;
 		lk->flags |= LOCK_FLAGS_ON_LIST;
-		strbuf_init(&lk->filename, PATH_MAX);
+		strbuf_init(&lk->filename, 0);
 		lk->next = lock_file_list;
 		lock_file_list = lk;
 	}
 
 	strbuf_addstr(&lk->filename, path);
-	if (!(flags & LOCK_NODEREF)) {
-		resolve_symlink(lk->filename.buf, lk->filename.alloc);
-		strbuf_setlen(&lk->filename, strlen(lk->filename.buf));
-	}
+	if (!(flags & LOCK_NODEREF))
+		resolve_symlink(&lk->filename);
 	strbuf_addstr(&lk->filename, ".lock");
 	lk->fd = open(lk->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666);
 	if (lk->fd < 0) {
-- 
1.9.1
Previous: Michael HaggertyNext: Michael Haggerty
Message 35 of 37 in “Lockfile correctness and refactoring”
  1. 00/25 Lockfile correctness and refactoringMichael Haggerty, Apr 6, 2014
  2. 01/25 api-lockfile: expand the documentationMichael Haggerty, Apr 6, 2014
  3. Jeff KingApr 7, 2014
  4. 02/25 unable_to_lock_die(): rename function from unable_to_lock_index_die()Michael Haggerty, Apr 6, 2014
  5. Jeff KingApr 7, 2014
  6. Michael HaggertyApr 8, 2014
  7. 03/25 rollback_lock_file(): do not clear filename redundantlyMichael Haggerty, Apr 6, 2014
  8. 04/25 rollback_lock_file(): set fd to -1Michael Haggerty, Apr 6, 2014
  9. 05/25 lockfile: unlock file if lockfile permissions cannot be adjustedMichael Haggerty, Apr 6, 2014
  10. 06/25 hold_lock_file_for_append(): release lock on errorsMichael Haggerty, Apr 6, 2014
  11. 07/25 lock_file(): always add lock_file object to lock_file_listMichael Haggerty, Apr 6, 2014
  12. 08/25 struct lock_file: replace on_list field with flags fieldMichael Haggerty, Apr 6, 2014
  13. 09/25 lockfile.c: document the various states of lock_file objectsMichael Haggerty, Apr 6, 2014
  14. 10/25 lockfile: define a constant LOCK_SUFFIX_LENMichael Haggerty, Apr 6, 2014
  15. 11/25 delete_ref_loose(): don't muck around in the lock_file's filenameMichael Haggerty, Apr 6, 2014
  16. 12/25 prepare_index(): declare return value to be (const char *)Michael Haggerty, Apr 6, 2014
  17. 13/25 write_packed_entry_fn(): convert cb_data into a (const int *)Michael Haggerty, Apr 6, 2014
  18. 14/25 lock_file(): exit early if lockfile cannot be openedMichael Haggerty, Apr 6, 2014
  19. 15/25 remove_lock_file(): call rollback_lock_file()Michael Haggerty, Apr 6, 2014
  20. 16/25 commit_lock_file(): inline temporary variableMichael Haggerty, Apr 6, 2014
  21. 17/25 commit_lock_file(): make committing an unlocked lockfile a NOPMichael Haggerty, Apr 6, 2014
  22. Jeff KingApr 7, 2014
  23. Michael HaggertyApr 14, 2014
  24. 18/25 lockfile: avoid transitory invalid statesMichael Haggerty, Apr 6, 2014
  25. Johannes SixtApr 7, 2014
  26. Michael HaggertyApr 7, 2014
  27. Johannes SixtApr 7, 2014
  28. Michael HaggertyApr 7, 2014
  29. Jeff KingApr 7, 2014
  30. 19/25 try_merge_strategy(): remove redundant lock_file allocationMichael Haggerty, Apr 6, 2014
  31. 20/25 try_merge_strategy(): use a statically-allocated lock_file objectMichael Haggerty, Apr 6, 2014
  32. 21/25 commit_lock_file(): use a strbuf to manage temporary spaceMichael Haggerty, Apr 6, 2014
  33. 22/25 Change lock_file::filename into a strbufMichael Haggerty, Apr 6, 2014
  34. 23/25 resolve_symlink(): use a strbuf for internal scratch spaceMichael Haggerty, Apr 6, 2014
  35. 24/25 resolve_symlink(): take a strbuf parameterMichael Haggerty, Apr 6, 2014
  36. 25/25 trim_last_path_elm(): replace last_path_elm()Michael Haggerty, Apr 6, 2014
  37. Jeff KingApr 7, 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.