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

Re: [PATCH/RFC] introduce strbuf_addpath()

From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date
Nov 18, 2011, 01:42 UTC
Message-ID
<20111118014235.GA10917@tre>
In-Reply-To
<20111116215004.GA29872@elie.hsd1.il.comcast.net>
On Wed, Nov 16, 2011 at 03:50:04PM -0600, Jonathan Nieder wrote:
Show 32 quoted lines
> strbuf_addpath() is like git_path_unsafe(), except instead of
> returning its own buffer, it appends its result to a buffer provided
> by the caller.
> 
> Benefits:
> 
>  - Since it uses a caller-supplied buffer, unlike git_path_unsafe(),
>    there is no risk that one call will clobber the result from
>    another.
> 
>  - Unlike git_pathdup(), it does not need to waste time allocating
>    memory in the middle of your tight loop over refs.
> 
>  - The size of the result is not limited to PATH_MAX.
> 
> Caveat: the size of its result is not limited to PATH_MAX.  Existing
> code might be relying on git_path*() to produce a result that is safe
> to copy to a PATH_MAX-sized buffer.  Be careful.
> 
> This patch introduces the strbuf_addpath() function and converts a few
> existing users of the strbuf_addstr(git_path(...)) idiom to
> demonstrate the API.
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Jonathan Nieder wrote:
> 
> > I think if I ran the world, the fundamental operation would be
> > strbuf_addpath().
> 
> Like this, maybe.
>

If I ran the world, I would change get_pathname() to return "struct strbuf *" instead and change "char *git_path(..)" to "const char *git_path(...)". Code paths that want to modify git_path() return value could just use strbuf_addpath()

I did try to turn git_path to return const char *, the following patch seems to make it build without any warnings

-- 8< --
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index c6bc8eb..fd7c682 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1041,7 +1041,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 	if (args.depth > 0) {
 		struct cache_time mtime;
 		struct strbuf sb = STRBUF_INIT;
-		char *shallow = git_path("shallow");
+		const char *shallow = git_path("shallow");
 		int fd;
 
 		mtime.sec = st.st_mtime;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 91731b9..261f1a0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -367,7 +367,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	char note[1024];
 	const char *what, *kind;
 	struct ref *rm;
-	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+	const char *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+	char *url;
 
 	fp = fopen(filename, "a");
 	if (!fp)
@@ -647,7 +648,7 @@ static void check_not_current_branch(struct ref *ref_map)
 
 static int truncate_fetch_head(void)
 {
-	char *filename = git_path("FETCH_HEAD");
+	const char *filename = git_path("FETCH_HEAD");
 	FILE *fp = fopen(filename, "w");
 
 	if (!fp)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0e0e17a..f9624d7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -215,12 +215,12 @@ static void check_unreachable_object(struct object *obj)
 		printf("dangling %s %s\n", typename(obj->type),
 		       sha1_to_hex(obj->sha1));
 		if (write_lost_and_found) {
-			char *filename = git_path("lost-found/%s/%s",
+			const char *filename = git_path("lost-found/%s/%s",
 				obj->type == OBJ_COMMIT ? "commit" : "other",
 				sha1_to_hex(obj->sha1));
 			FILE *f;
 
-			if (safe_create_leading_directories(filename)) {
+			if (safe_create_leading_directories_const(filename)) {
 				error("Could not create lost-found");
 				return;
 			}
diff --git a/builtin/remote.c b/builtin/remote.c
index 583eec9..0662d37 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -587,7 +587,7 @@ static int migrate_file(struct remote *remote)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int i;
-	char *path = NULL;
+	const char *path = NULL;
 
 	strbuf_addf(&buf, "remote.%s.url", remote->name);
 	for (i = 0; i < remote->url_nr; i++)
diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c..4293a9f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -403,7 +403,7 @@ static void dump_marks_helper(FILE *, uintmax_t, struct mark_set *);
 
 static void write_crash_report(const char *err)
 {
-	char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
+	const char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
 	FILE *rpt = fopen(loc, "w");
 	struct branch *b;
 	unsigned long lu;
diff --git a/notes-merge.c b/notes-merge.c
index e33c2c9..8da2e0a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -288,7 +288,7 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
 				    "(%s exists).", git_path("NOTES_MERGE_*"));
 		}
 
-		if (safe_create_leading_directories(git_path(
+		if (safe_create_leading_directories_const(git_path(
 				NOTES_MERGE_WORKTREE "/.test")))
 			die_errno("unable to create directory %s",
 				  git_path(NOTES_MERGE_WORKTREE));
@@ -303,8 +303,8 @@ static void write_buf_to_worktree(const unsigned char *obj,
 				  const char *buf, unsigned long size)
 {
 	int fd;
-	char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
-	if (safe_create_leading_directories(path))
+	const char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
+	if (safe_create_leading_directories_const(path))
 		die_errno("unable to create directory for '%s'", path);
 	if (file_exists(path))
 		die("found existing file at '%s'", path);
diff --git a/refs.c b/refs.c
index 62d8a37..7469cf1 100644
--- a/refs.c
+++ b/refs.c
@@ -1189,7 +1189,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 
 static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int flags, int *type_p)
 {
-	char *ref_file;
+	const char *ref_file;
 	const char *orig_ref = ref;
 	struct ref_lock *lock;
 	int last_errno = 0;
@@ -1250,7 +1250,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
 	if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
 		lock->force_write = 1;
 
-	if (safe_create_leading_directories(ref_file)) {
+	if (safe_create_leading_directories_const(ref_file)) {
 		last_errno = errno;
 		error("unable to create directory for %s", ref_file);
 		goto error_return;
@@ -1411,7 +1411,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 		}
 	}
 
-	if (log && safe_create_leading_directories(git_path("logs/%s", newref))) {
+	if (log && safe_create_leading_directories_const(git_path("logs/%s", newref))) {
 		error("unable to create directory for %s", newref);
 		goto rollback;
 	}
-- 8< --
Previous: Jonathan NiederNext: Junio C Hamano
Message 32 of 47 in “Sequencer: working around historical mistakes”
  1. 0/5 Sequencer: working around historical mistakesRamkumar Ramachandra, Nov 5, 2011
  2. 1/5 sequencer: factor code out of revert builtinRamkumar Ramachandra, Nov 5, 2011
  3. Jonathan NiederNov 6, 2011
  4. Ramkumar RamachandraNov 13, 2011
  5. Junio C HamanoNov 13, 2011
  6. Ramkumar RamachandraNov 15, 2011
  7. Miles BaderNov 15, 2011
  8. Jonathan NiederNov 15, 2011
  9. 2/5 sequencer: remove CHERRY_PICK_HEAD with sequencer stateRamkumar Ramachandra, Nov 5, 2011
  10. Jonathan NiederNov 6, 2011
  11. 3/5 sequencer: sequencer state is useless without todoRamkumar Ramachandra, Nov 5, 2011
  12. Jonathan NiederNov 6, 2011
  13. Ramkumar RamachandraNov 13, 2011
  14. Junio C HamanoNov 13, 2011
  15. Ramkumar RamachandraNov 15, 2011
  16. Jonathan NiederNov 15, 2011
  17. Junio C HamanoNov 15, 2011
  18. Ramkumar RamachandraNov 16, 2011
  19. Junio C HamanoNov 16, 2011
  20. 0/3 avoiding unintended consequences of git_path() usageJonathan Nieder, Nov 16, 2011
  21. 1/3 do not let git_path clobber errno when reporting errorsJonathan Nieder, Nov 16, 2011
  22. 2/3 Bigfile: dynamically allocate buffer for marks file nameJonathan Nieder, Nov 16, 2011
  23. 3/3 rename git_path() to git_path_unsafe()Jonathan Nieder, Nov 16, 2011
  24. Junio C HamanoNov 17, 2011
  25. Jonathan NiederNov 17, 2011
  26. Nguyen Thai Ngoc DuyNov 16, 2011
  27. Nguyen Thai Ngoc DuyNov 16, 2011
  28. Jonathan NiederNov 16, 2011
  29. Nguyen Thai Ngoc DuyNov 16, 2011
  30. Ramsay JonesNov 19, 2011
  31. introduce strbuf_addpath()Jonathan Nieder, Nov 16, 2011
  32. Nguyen Thai Ngoc DuyNov 18, 2011
  33. Junio C HamanoNov 16, 2011
  34. Ramkumar RamachandraNov 16, 2011
  35. Nguyen Thai Ngoc DuyNov 16, 2011
  36. Michael HaggertyNov 16, 2011
  37. Nguyen Thai Ngoc DuyNov 18, 2011
  38. 4/5 sequencer: handle single commit pick separatelyRamkumar Ramachandra, Nov 5, 2011
  39. Jonathan NiederNov 6, 2011
  40. 5/5 sequencer: revert d3f4628eRamkumar Ramachandra, Nov 5, 2011
  41. Jonathan NiederNov 6, 2011
  42. Junio C HamanoNov 6, 2011
  43. Ramkumar RamachandraNov 7, 2011
  44. Ramkumar RamachandraNov 12, 2011
  45. Jonathan NiederNov 12, 2011
  46. Jonathan NiederNov 5, 2011
  47. Ramkumar RamachandraNov 13, 2011

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.