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

[PATCH next v2] log_ref_setup: don't return stack-allocated array

From
Thomas Rast <trast@student.ethz.ch>
Date
Jun 10, 2010, 12:54 UTC
Message-ID
<47daf53b6b2cc25cc013c5f2183e309a671dc9d3.1276174233.git.trast@student.ethz.ch>
In-Reply-To
<e888313d5a782585f4a5e7ee8914302953c187e2.1276173576.git.trast@student.ethz.ch>

859c301 (refs: split log_ref_write logic into log_ref_setup, 2010-05-21) refactors the stack allocation of the log_file array into the new log_ref_setup() function, but passes it back to the caller.

Since the original intent seems to have been to split the work between log_ref_setup and log_ref_write, make it the caller's responsibility to allocate the buffer.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

Sorry for the first one, that was completely botched and didn't even compile.

This one does, and as an added bonus also passes some tests.
 builtin/checkout.c |    4 ++--
 refs.c             |   26 ++++++++++++--------------
 refs.h             |    2 +-
 3 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5107eda..1994be9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -496,12 +496,12 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 		if (opts->new_orphan_branch) {
 			if (opts->new_branch_log && !log_all_ref_updates) {
 				int temp;
-				char *log_file;
+				char log_file[PATH_MAX];
 				char *ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
 
 				temp = log_all_ref_updates;
 				log_all_ref_updates = 1;
-				if (log_ref_setup(ref_name, &log_file)) {
+				if (log_ref_setup(ref_name, log_file, sizeof(log_file))) {
 					fprintf(stderr, "Can not do reflog for '%s'\n",
 					    opts->new_orphan_branch);
 					log_all_ref_updates = temp;
diff --git a/refs.c b/refs.c
index 3436649..6f486ae 100644
--- a/refs.c
+++ b/refs.c
@@ -1262,43 +1262,41 @@ static int copy_msg(char *buf, const char *msg)
 	return cp - buf;
 }
 
-int log_ref_setup(const char *ref_name, char **log_file)
+int log_ref_setup(const char *ref_name, char *logfile, int bufsize)
 {
 	int logfd, oflags = O_APPEND | O_WRONLY;
-	char logfile[PATH_MAX];
 
-	git_snpath(logfile, sizeof(logfile), "logs/%s", ref_name);
-	*log_file = logfile;
+	git_snpath(logfile, bufsize, "logs/%s", ref_name);
 	if (log_all_ref_updates &&
 	    (!prefixcmp(ref_name, "refs/heads/") ||
 	     !prefixcmp(ref_name, "refs/remotes/") ||
 	     !prefixcmp(ref_name, "refs/notes/") ||
 	     !strcmp(ref_name, "HEAD"))) {
-		if (safe_create_leading_directories(*log_file) < 0)
+		if (safe_create_leading_directories(logfile) < 0)
 			return error("unable to create directory for %s",
-				     *log_file);
+				     logfile);
 		oflags |= O_CREAT;
 	}
 
-	logfd = open(*log_file, oflags, 0666);
+	logfd = open(logfile, oflags, 0666);
 	if (logfd < 0) {
 		if (!(oflags & O_CREAT) && errno == ENOENT)
 			return 0;
 
 		if ((oflags & O_CREAT) && errno == EISDIR) {
-			if (remove_empty_directories(*log_file)) {
+			if (remove_empty_directories(logfile)) {
 				return error("There are still logs under '%s'",
-					     *log_file);
+					     logfile);
 			}
-			logfd = open(*log_file, oflags, 0666);
+			logfd = open(logfile, oflags, 0666);
 		}
 
 		if (logfd < 0)
 			return error("Unable to append to %s: %s",
-				     *log_file, strerror(errno));
+				     logfile, strerror(errno));
 	}
 
-	adjust_shared_perm(*log_file);
+	adjust_shared_perm(logfile);
 	close(logfd);
 	return 0;
 }
@@ -1309,14 +1307,14 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 	int logfd, result, written, oflags = O_APPEND | O_WRONLY;
 	unsigned maxlen, len;
 	int msglen;
-	char *log_file;
+	char log_file[PATH_MAX];
 	char *logrec;
 	const char *committer;
 
 	if (log_all_ref_updates < 0)
 		log_all_ref_updates = !is_bare_repository();
 
-	result = log_ref_setup(ref_name, &log_file);
+	result = log_ref_setup(ref_name, log_file, sizeof(log_file));
 	if (result)
 		return result;
 
diff --git a/refs.h b/refs.h
index 594c9d9..762ce50 100644
--- a/refs.h
+++ b/refs.h
@@ -69,7 +69,7 @@ extern void unlock_ref(struct ref_lock *lock);
 extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
 
 /** Setup reflog before using. **/
-int log_ref_setup(const char *ref_name, char **log_file);
+int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
 
 /** Reads log for the value of ref during at_time. **/
 extern int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *sha1, char **msg, unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt);
-- 
1.7.1.553.ga798e
Previous: Thomas RastNext: Erick Mattos
Message 2 of 11 in “log_ref_setup: don't return stack-allocated array”
  1. log_ref_setup: don't return stack-allocated arrayThomas Rast, Jun 10, 2010
  2. log_ref_setup: don't return stack-allocated arrayThomas Rast, Jun 10, 2010
  3. Erick MattosJun 10, 2010
  4. Thomas RastJun 10, 2010
  5. Erick MattosJun 10, 2010
  6. Jeff KingJun 11, 2010
  7. Erick MattosJun 11, 2010
  8. Ævar Arnfjörð BjarmasonJun 10, 2010
  9. check_aliased_update: strcpy() instead of strcat() to copyThomas Rast, Jun 10, 2010
  10. Ævar Arnfjörð BjarmasonJun 10, 2010
  11. Jay SoffianJun 10, 2010

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.