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

[PATCH v7 30/38] resolve_symlink(): take a strbuf parameter

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Oct 1, 2014, 10:28 UTC
Message-ID
<1412159322-2622-31-git-send-email-mhagger@alum.mit.edu>
In-Reply-To
<1412159322-2622-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.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 lockfile.c | 57 ++++++++++++++++++++++-----------------------------------
 1 file changed, 22 insertions(+), 35 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index cc9b9cb..5f5bcff 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -109,58 +109,47 @@ 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;
 }
 
 /* Make sure errno contains a meaningful value on error */
 static int lock_file(struct lock_file *lk, const char *path, int flags)
 {
+	size_t pathlen = strlen(path);
+
 	if (!lock_file_list) {
 		/* One-time initialization */
 		sigchain_push_common(remove_lock_file_on_signal);
@@ -175,7 +164,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 		lk->fd = -1;
 		lk->active = 0;
 		lk->owner = 0;
-		strbuf_init(&lk->filename, PATH_MAX);
+		strbuf_init(&lk->filename, pathlen + LOCK_SUFFIX_LEN);
 		lk->next = lock_file_list;
 		lock_file_list = lk;
 		lk->on_list = 1;
@@ -185,11 +174,9 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 		    path);
 	}
 
-	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));
-	}
+	strbuf_add(&lk->filename, path, pathlen);
+	if (!(flags & LOCK_NODEREF))
+		resolve_symlink(&lk->filename);
 	strbuf_addstr(&lk->filename, LOCK_SUFFIX);
 	lk->fd = open(lk->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666);
 	if (lk->fd < 0) {
-- 
2.1.0
Previous: Michael HaggertyNext: Michael Haggerty
Message 34 of 43 in “Lockfile correctness and refactoring”
  1. 00/38 Lockfile correctness and refactoringMichael Haggerty, Oct 1, 2014
  2. 01/38 unable_to_lock_die(): rename function from unable_to_lock_index_die()Michael Haggerty, Oct 1, 2014
  3. 02/38 api-lockfile: revise and expand the documentationMichael Haggerty, Oct 1, 2014
  4. 03/38 close_lock_file(): exit (successfully) if file is already closedMichael Haggerty, Oct 1, 2014
  5. 04/38 rollback_lock_file(): do not clear filename redundantlyMichael Haggerty, Oct 1, 2014
  6. 05/38 rollback_lock_file(): exit early if lock is not activeMichael Haggerty, Oct 1, 2014
  7. 06/38 rollback_lock_file(): set fd to -1Michael Haggerty, Oct 1, 2014
  8. 07/38 lockfile: unlock file if lockfile permissions cannot be adjustedMichael Haggerty, Oct 1, 2014
  9. 08/38 hold_lock_file_for_append(): release lock on errorsMichael Haggerty, Oct 1, 2014
  10. 09/38 lock_file(): always initialize and register lock_file objectMichael Haggerty, Oct 1, 2014
  11. René ScharfeOct 1, 2014
  12. Michael HaggertyOct 1, 2014
  13. Junio C HamanoOct 1, 2014
  14. 10/38 lockfile.c: document the various states of lock_file objectsMichael Haggerty, Oct 1, 2014
  15. 11/38 cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LENMichael Haggerty, Oct 1, 2014
  16. 12/38 delete_ref_loose(): don't muck around in the lock_file's filenameMichael Haggerty, Oct 1, 2014
  17. 13/38 prepare_index(): declare return value to be (const char *)Michael Haggerty, Oct 1, 2014
  18. 14/38 lock_file(): exit early if lockfile cannot be openedMichael Haggerty, Oct 1, 2014
  19. 15/38 remove_lock_file(): call rollback_lock_file()Michael Haggerty, Oct 1, 2014
  20. 16/38 commit_lock_file(): inline temporary variableMichael Haggerty, Oct 1, 2014
  21. 17/38 commit_lock_file(): die() if called for unlocked lockfile objectMichael Haggerty, Oct 1, 2014
  22. 18/38 close_lock_file(): if close fails, roll backMichael Haggerty, Oct 1, 2014
  23. 19/38 commit_lock_file(): rollback lock file on failure to renameMichael Haggerty, Oct 1, 2014
  24. 20/38 api-lockfile: document edge casesMichael Haggerty, Oct 1, 2014
  25. 21/38 dump_marks(): remove a redundant call to rollback_lock_file()Michael Haggerty, Oct 1, 2014
  26. 22/38 git_config_set_multivar_in_file(): avoid call to rollback_lock_file()Michael Haggerty, Oct 1, 2014
  27. 23/38 lockfile: avoid transitory invalid statesMichael Haggerty, Oct 1, 2014
  28. 24/38 struct lock_file: declare some fields volatileMichael Haggerty, Oct 1, 2014
  29. 25/38 try_merge_strategy(): remove redundant lock_file allocationMichael Haggerty, Oct 1, 2014
  30. 26/38 try_merge_strategy(): use a statically-allocated lock_file objectMichael Haggerty, Oct 1, 2014
  31. 27/38 commit_lock_file(): use a strbuf to manage temporary spaceMichael Haggerty, Oct 1, 2014
  32. 28/38 Change lock_file::filename into a strbufMichael Haggerty, Oct 1, 2014
  33. 29/38 resolve_symlink(): use a strbuf for internal scratch spaceMichael Haggerty, Oct 1, 2014
  34. 30/38 resolve_symlink(): take a strbuf parameterMichael Haggerty, Oct 1, 2014
  35. 31/38 trim_last_path_component(): replace last_path_elm()Michael Haggerty, Oct 1, 2014
  36. 32/38 Extract a function commit_lock_file_to()Michael Haggerty, Oct 1, 2014
  37. 33/38 Rename LOCK_NODEREF to LOCK_NO_DEREFMichael Haggerty, Oct 1, 2014
  38. 34/38 lockfile.c: rename static functionsMichael Haggerty, Oct 1, 2014
  39. 35/38 get_locked_file_path(): new functionMichael Haggerty, Oct 1, 2014
  40. 36/38 hold_lock_file_for_append(): restore errno before returningMichael Haggerty, Oct 1, 2014
  41. 37/38 Move read_index() definition to read-cache.cMichael Haggerty, Oct 1, 2014
  42. 38/38 lockfile.h: extract new header file for the functions in lockfile.cMichael Haggerty, Oct 1, 2014
  43. Junio C HamanoOct 1, 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.