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

[PATCH 2/2] entry.c: convert write_entry to use strbuf

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Oct 23, 2013, 12:55 UTC
Message-ID
<1382532907-30561-2-git-send-email-pclouds@gmail.com>
In-Reply-To
<1382532907-30561-1-git-send-email-pclouds@gmail.com>

The strcpy call in open_output_fd() implies that the output buffer must be at least 25 chars long. And it's true. The only caller that can trigger that code is checkout-index, which has the buffer of PATH_MAX chars (and any systems that have PATH_MAX shorter than 25 chars are just insane).

But in order to say that, one has to walk through a dozen of functions. Just convert it to strbuf to avoid the constraint and confusion.

Although my original motivation was simpler than that. I just wanted to change "char *path" to "const char *path" in checkout_entry() to make sure no funny business regarding "path" in that function.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/checkout-index.c | 19 ++++++++++++-------
 cache.h                  |  2 +-
 entry.c                  | 29 ++++++++++++++++-------------
 3 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 69e167b..6d88c0c 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -14,7 +14,12 @@
 static int line_termination = '\n';
 static int checkout_stage; /* default to checkout stage0 */
 static int to_tempfile;
-static char topath[4][PATH_MAX + 1];
+static struct strbuf topath[4] = {
+	STRBUF_INIT,
+	STRBUF_INIT,
+	STRBUF_INIT,
+	STRBUF_INIT
+};
 
 static struct checkout state;
 
@@ -26,19 +31,19 @@ static void write_tempfile_record(const char *name, int prefix_length)
 		for (i = 1; i < 4; i++) {
 			if (i > 1)
 				putchar(' ');
-			if (topath[i][0])
-				fputs(topath[i], stdout);
+			if (topath[i].len)
+				fputs(topath[i].buf, stdout);
 			else
 				putchar('.');
 		}
 	} else
-		fputs(topath[checkout_stage], stdout);
+		fputs(topath[checkout_stage].buf, stdout);
 
 	putchar('\t');
 	write_name_quoted(name + prefix_length, stdout, line_termination);
 
 	for (i = 0; i < 4; i++) {
-		topath[i][0] = 0;
+		strbuf_reset(&topath[i]);
 	}
 }
 
@@ -65,7 +70,7 @@ static int checkout_file(const char *name, int prefix_length)
 			continue;
 		did_checkout = 1;
 		if (checkout_entry(ce, &state,
-		    to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)
+		    to_tempfile ? &topath[ce_stage(ce)] : NULL) < 0)
 			errs++;
 	}
 
@@ -109,7 +114,7 @@ static void checkout_all(const char *prefix, int prefix_length)
 				write_tempfile_record(last_ce->name, prefix_length);
 		}
 		if (checkout_entry(ce, &state,
-		    to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)
+		    to_tempfile ? &topath[ce_stage(ce)] : NULL) < 0)
 			errs++;
 		last_ce = ce;
 	}
diff --git a/cache.h b/cache.h
index 51d6602..276182f 100644
--- a/cache.h
+++ b/cache.h
@@ -962,7 +962,7 @@ struct checkout {
 		 refresh_cache:1;
 };
 
-extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
+extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, struct strbuf *topath);
 
 struct cache_def {
 	char path[PATH_MAX + 1];
diff --git a/entry.c b/entry.c
index d955af5..a76942d 100644
--- a/entry.c
+++ b/entry.c
@@ -92,15 +92,15 @@ static void *read_blob_entry(const struct cache_entry *ce, unsigned long *size)
 	return NULL;
 }
 
-static int open_output_fd(char *path, const struct cache_entry *ce, int to_tempfile)
+static int open_output_fd(struct strbuf *path, const struct cache_entry *ce, int to_tempfile)
 {
 	int symlink = (ce->ce_mode & S_IFMT) != S_IFREG;
 	if (to_tempfile) {
-		strcpy(path, symlink
-		       ? ".merge_link_XXXXXX" : ".merge_file_XXXXXX");
-		return mkstemp(path);
+		strbuf_reset(path);
+		strbuf_addstr(path, symlink ? ".merge_link_XXXXXX" : ".merge_file_XXXXXX");
+		return mkstemp(path->buf);
 	} else {
-		return create_file(path, !symlink ? ce->ce_mode : 0666);
+		return create_file(path->buf, !symlink ? ce->ce_mode : 0666);
 	}
 }
 
@@ -115,7 +115,7 @@ static int fstat_output(int fd, const struct checkout *state, struct stat *st)
 	return 0;
 }
 
-static int streaming_write_entry(const struct cache_entry *ce, char *path,
+static int streaming_write_entry(const struct cache_entry *ce, struct strbuf *path,
 				 struct stream_filter *filter,
 				 const struct checkout *state, int to_tempfile,
 				 int *fstat_done, struct stat *statbuf)
@@ -132,12 +132,12 @@ static int streaming_write_entry(const struct cache_entry *ce, char *path,
 	result |= close(fd);
 
 	if (result)
-		unlink(path);
+		unlink(path->buf);
 	return result;
 }
 
 static int write_entry(struct cache_entry *ce,
-		       char *path, const struct checkout *state, int to_tempfile)
+		       struct strbuf *path_buf, const struct checkout *state, int to_tempfile)
 {
 	unsigned int ce_mode_s_ifmt = ce->ce_mode & S_IFMT;
 	int fd, ret, fstat_done = 0;
@@ -146,15 +146,17 @@ static int write_entry(struct cache_entry *ce,
 	unsigned long size;
 	size_t wrote, newsize = 0;
 	struct stat st;
+	const char *path;
 
 	if (ce_mode_s_ifmt == S_IFREG) {
 		struct stream_filter *filter = get_stream_filter(ce->name, ce->sha1);
 		if (filter &&
-		    !streaming_write_entry(ce, path, filter,
+		    !streaming_write_entry(ce, path_buf, filter,
 					   state, to_tempfile,
 					   &fstat_done, &st))
 			goto finish;
 	}
+	path = path_buf->buf;
 
 	switch (ce_mode_s_ifmt) {
 	case S_IFREG:
@@ -183,7 +185,8 @@ static int write_entry(struct cache_entry *ce,
 			size = newsize;
 		}
 
-		fd = open_output_fd(path, ce, to_tempfile);
+		fd = open_output_fd(path_buf, ce, to_tempfile);
+		path = path_buf->buf;
 		if (fd < 0) {
 			free(new);
 			return error("unable to create file %s (%s)",
@@ -235,10 +238,10 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)
 }
 
 int checkout_entry(struct cache_entry *ce,
-		   const struct checkout *state, char *topath)
+		   const struct checkout *state, struct strbuf *topath)
 {
 	static struct strbuf path_buf = STRBUF_INIT;
-	char *path;
+	const char *path;
 	struct stat st;
 	int len;
 
@@ -278,5 +281,5 @@ int checkout_entry(struct cache_entry *ce,
 	} else if (state->not_new)
 		return 0;
 	create_directories(path, len, state);
-	return write_entry(ce, path, state, 0);
+	return write_entry(ce, &path_buf, state, 0);
 }

-- 
1.8.2.83.gc99314b

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
Previous: Nguyễn Thái Ngọc DuyNext: Junio C Hamano
Message 26 of 44 in “mingw-multibyte: fix memory acces violation and path length limits.”
  1. mingw-multibyte: fix memory acces violation and path length limits.Wataru Noguchi, Sep 28, 2013
  2. Johannes SchindelinSep 28, 2013
  3. Wataru NoguchiSep 29, 2013
  4. Stefan BellerSep 29, 2013
  5. Wataru NoguchiOct 1, 2013
  6. René ScharfeSep 30, 2013
  7. Erik Faye-LundSep 30, 2013
  8. Wataru NoguchiOct 1, 2013
  9. Wataru NoguchiOct 2, 2013
  10. Antoine PelisseOct 3, 2013
  11. Erik Faye-LundOct 3, 2013
  12. Wataru NoguchiOct 5, 2013
  13. Prevent buffer overflows when path is too bigAntoine Pelisse, Oct 19, 2013
  14. Torsten BögershausenOct 20, 2013
  15. Ondřej BílkaOct 20, 2013
  16. Torsten BögershausenOct 20, 2013
  17. Ondřej BílkaOct 20, 2013
  18. Duy NguyenOct 20, 2013
  19. Antoine PelisseOct 20, 2013
  20. Duy NguyenOct 21, 2013
  21. Johannes SixtOct 21, 2013
  22. Erik Faye-LundOct 21, 2013
  23. Jeff KingOct 21, 2013
  24. Jeff KingOct 21, 2013
  25. 1/2 entry.c: convert checkout_entry to use strbufNguyễn Thái Ngọc Duy, Oct 23, 2013
  26. 2/2 entry.c: convert write_entry to use strbufNguyễn Thái Ngọc Duy, Oct 23, 2013
  27. Junio C HamanoOct 23, 2013
  28. Duy NguyenOct 24, 2013
  29. Junio C HamanoOct 24, 2013
  30. Duy NguyenOct 24, 2013
  31. Antoine PelisseOct 23, 2013
  32. Duy NguyenOct 23, 2013
  33. Antoine PelisseOct 23, 2013
  34. Jeff KingOct 23, 2013
  35. Erik Faye-LundOct 23, 2013
  36. Jeff KingOct 23, 2013
  37. Junio C HamanoOct 23, 2013
  38. Jeff KingOct 23, 2013
  39. entry.c: convert checkout_entry to use strbufNguyễn Thái Ngọc Duy, Oct 24, 2013
  40. Duy NguyenOct 23, 2013
  41. Prevent buffer overflows when path is too longAntoine Pelisse, Nov 26, 2013
  42. Junio C HamanoNov 26, 2013
  43. Antoine PelisseNov 29, 2013
  44. Prevent buffer overflows when path is too longAntoine Pelisse, Dec 14, 2013

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.