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

[PATCH 4/6] pack-objects.c: fix some global variable abuse and memory leaks

From
Nicolas Pitre <nico@cam.org>
Date
Oct 17, 2007, 01:55 UTC
Message-ID
<1192586150-13743-5-git-send-email-nico@cam.org>
In-Reply-To
<1192586150-13743-4-git-send-email-nico@cam.org>

To keep things well layered, sha1close() now returns the file descriptor when it doesn't close the file.

An ugly cast was added to the return of write_idx_file() to avoid a warning. A proper fix will come separately.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 builtin-pack-objects.c |   29 +++++++++++++++--------------
 csum-file.c            |   23 ++++++++++++++---------
 2 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index d729cb7..933c252 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -65,8 +65,6 @@ static int no_reuse_delta, no_reuse_object, keep_unreachable;
 static int local;
 static int incremental;
 static int allow_ofs_delta;
-static const char *pack_tmp_name, *idx_tmp_name;
-static char tmpname[PATH_MAX];
 static const char *base_name;
 static int progress = 1;
 static int window = 10;
@@ -587,12 +585,6 @@ static off_t write_one(struct sha1file *f,
 	return offset + size;
 }
 
-static int open_object_dir_tmp(const char *path)
-{
-    snprintf(tmpname, sizeof(tmpname), "%s/%s", get_object_directory(), path);
-    return xmkstemp(tmpname);
-}
-
 /* forward declaration for write_pack_file */
 static int adjust_perm(const char *path, mode_t mode);
 
@@ -611,11 +603,16 @@ static void write_pack_file(void)
 
 	do {
 		unsigned char sha1[20];
+		char *pack_tmp_name = NULL;
 
 		if (pack_to_stdout) {
 			f = sha1fd(1, "<stdout>");
 		} else {
-			int fd = open_object_dir_tmp("tmp_pack_XXXXXX");
+			char tmpname[PATH_MAX];
+			int fd;
+			snprintf(tmpname, sizeof(tmpname),
+				 "%s/tmp_pack_XXXXXX", get_object_directory());
+			fd = xmkstemp(tmpname);
 			pack_tmp_name = xstrdup(tmpname);
 			f = sha1fd(fd, pack_tmp_name);
 		}
@@ -643,19 +640,21 @@ static void write_pack_file(void)
 		if (pack_to_stdout || nr_written == nr_remaining) {
 			sha1close(f, sha1, 1);
 		} else {
-			sha1close(f, sha1, 0);
-			fixup_pack_header_footer(f->fd, sha1, pack_tmp_name, nr_written);
-			close(f->fd);
+			int fd = sha1close(f, NULL, 0);
+			fixup_pack_header_footer(fd, sha1, pack_tmp_name, nr_written);
+			close(fd);
 		}
 
 		if (!pack_to_stdout) {
 			mode_t mode = umask(0);
+			char *idx_tmp_name, tmpname[PATH_MAX];
 
 			umask(mode);
 			mode = 0444 & ~mode;
 
-			idx_tmp_name = write_idx_file(NULL,
-				(struct pack_idx_entry **) written_list, nr_written, sha1);
+			idx_tmp_name = (char *) write_idx_file(NULL,
+					(struct pack_idx_entry **) written_list,
+					nr_written, sha1);
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
 				 base_name, sha1_to_hex(sha1));
 			if (adjust_perm(pack_tmp_name, mode))
@@ -672,6 +671,8 @@ static void write_pack_file(void)
 			if (rename(idx_tmp_name, tmpname))
 				die("unable to rename temporary index file: %s",
 				    strerror(errno));
+			free(idx_tmp_name);
+			free(pack_tmp_name);
 			puts(sha1_to_hex(sha1));
 		}
 
diff --git a/csum-file.c b/csum-file.c
index 9ab9971..9929991 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -31,22 +31,27 @@ static void sha1flush(struct sha1file *f, unsigned int count)
 
 int sha1close(struct sha1file *f, unsigned char *result, int final)
 {
+	int fd;
 	unsigned offset = f->offset;
 	if (offset) {
 		SHA1_Update(&f->ctx, f->buffer, offset);
 		sha1flush(f, offset);
 		f->offset = 0;
 	}
-	if (!final)
-		return 0;	/* only want to flush (no checksum write, no close) */
-	SHA1_Final(f->buffer, &f->ctx);
-	if (result)
-		hashcpy(result, f->buffer);
-	sha1flush(f, 20);
-	if (close(f->fd))
-		die("%s: sha1 file error on close (%s)", f->name, strerror(errno));
+	if (final) {
+		/* write checksum and close fd */
+		SHA1_Final(f->buffer, &f->ctx);
+		if (result)
+			hashcpy(result, f->buffer);
+		sha1flush(f, 20);
+		if (close(f->fd))
+			die("%s: sha1 file error on close (%s)",
+			    f->name, strerror(errno));
+		fd = 0;
+	} else
+		fd = f->fd;
 	free(f);
-	return 0;
+	return fd;
 }
 
 int sha1write(struct sha1file *f, void *buf, unsigned int count)
-- 
1.5.3.4.1212.gdb015
Previous: Nicolas PitreNext: Nicolas Pitre
Message 5 of 17 in “miscelaneous stuff”
  1. 0/6 miscelaneous stuffNicolas Pitre, Oct 17, 2007
  2. 1/6 more compact progress displayNicolas Pitre, Oct 17, 2007
  3. 2/6 cope with multiple line breaks within sideband progress messagesNicolas Pitre, Oct 17, 2007
  4. 3/6 pack-objects: no delta possible with only one object in the listNicolas Pitre, Oct 17, 2007
  5. 4/6 pack-objects.c: fix some global variable abuse and memory leaksNicolas Pitre, Oct 17, 2007
  6. 5/6 fix const issues with some functionsNicolas Pitre, Oct 17, 2007
  7. 6/6 fix for more minor memory leaksNicolas Pitre, Oct 17, 2007
  8. Deltifying? (was [PATCH 3/6] pack-objects: no delta possible...)Shawn O. Pearce, Oct 17, 2007
  9. Nicolas PitreOct 17, 2007
  10. Shawn O. PearceOct 17, 2007
  11. Nicolas PitreOct 17, 2007
  12. Karl HasselströmOct 17, 2007
  13. Nicolas PitreOct 17, 2007
  14. Shawn O. PearceOct 18, 2007
  15. Karl HasselströmOct 18, 2007
  16. Johannes SixtOct 17, 2007
  17. Shawn O. PearceOct 17, 2007

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.