Merge branch 'jk/prune-packed-server-info'

Code cleanup.

* jk/prune-packed-server-info:
  repack: call prune_packed_objects() and update_server_info() directly
  server-info: clean up after writing info/packs
  make update-server-info more robust
  prune-packed: fix minor memory leak

builtin/prune-packed.c

@@ -68,6 +68,7 @@ void prune_packed_objects(int opts)
 		rmdir(pathname.buf);
 	}
 	stop_progress(&progress);
+	strbuf_release(&pathname);
 }
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)

builtin/repack.c

@@ -377,6 +377,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	/* End of pack replacement. */
 
 	if (delete_redundant) {
+		int opts = 0;
 		sort_string_list(&names);
 		for_each_string_list_item(item, &existing_packs) {
 			char *sha1;
@@ -387,25 +388,13 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 			if (!string_list_has_string(&names, sha1))
 				remove_redundant_pack(packdir, item->string);
 		}
-		argv_array_push(&cmd_args, "prune-packed");
+		if (!quiet && isatty(2))
-		if (quiet)
+			opts |= PRUNE_PACKED_VERBOSE;
-			argv_array_push(&cmd_args, "--quiet");
+		prune_packed_objects(opts);
-
-		memset(&cmd, 0, sizeof(cmd));
-		cmd.argv = cmd_args.argv;
-		cmd.git_cmd = 1;
-		run_command(&cmd);
-		argv_array_clear(&cmd_args);
 	}
 
-	if (!no_update_server_info) {
+	if (!no_update_server_info)
-		argv_array_push(&cmd_args, "update-server-info");
+		update_server_info(0);
-		memset(&cmd, 0, sizeof(cmd));
-		cmd.argv = cmd_args.argv;
-		cmd.git_cmd = 1;
-		run_command(&cmd);
-		argv_array_clear(&cmd_args);
-	}
 	remove_temporary_files();
 	string_list_clear(&names, 0);
 	string_list_clear(&rollback, 0);

server-info.c

@@ -4,45 +4,80 @@
 #include "commit.h"
 #include "tag.h"
 
-/* refs */
+/*
-static FILE *info_ref_fp;
+ * Create the file "path" by writing to a temporary file and renaming
+ * it into place. The contents of the file come from "generate", which
+ * should return non-zero if it encounters an error.
+ */
+static int update_info_file(char *path, int (*generate)(FILE *))
+{
+	char *tmp = mkpathdup("%s_XXXXXX", path);
+	int ret = -1;
+	int fd = -1;
+	FILE *fp = NULL;
+
+	safe_create_leading_directories(path);
+	fd = mkstemp(tmp);
+	if (fd < 0)
+		goto out;
+	fp = fdopen(fd, "w");
+	if (!fp)
+		goto out;
+	ret = generate(fp);
+	if (ret)
+		goto out;
+	if (fclose(fp))
+		goto out;
+	if (adjust_shared_perm(tmp) < 0)
+		goto out;
+	if (rename(tmp, path) < 0)
+		goto out;
+	ret = 0;
+
+out:
+	if (ret) {
+		error("unable to update %s: %s", path, strerror(errno));
+		if (fp)
+			fclose(fp);
+		else if (fd >= 0)
+			close(fd);
+		unlink(tmp);
+	}
+	free(tmp);
+	return ret;
+}
 
 static int add_info_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
+	FILE *fp = cb_data;
 	struct object *o = parse_object(sha1);
 	if (!o)
 		return -1;
 
-	fprintf(info_ref_fp, "%s	%s\n", sha1_to_hex(sha1), path);
+	if (fprintf(fp, "%s	%s\n", sha1_to_hex(sha1), path) < 0)
+		return -1;
+
 	if (o->type == OBJ_TAG) {
 		o = deref_tag(o, path, 0);
 		if (o)
-			fprintf(info_ref_fp, "%s	%s^{}\n",
+			if (fprintf(fp, "%s	%s^{}\n",
-				sha1_to_hex(o->sha1), path);
+				sha1_to_hex(o->sha1), path) < 0)
+				return -1;
 	}
 	return 0;
 }
 
+static int generate_info_refs(FILE *fp)
+{
+	return for_each_ref(add_info_ref, fp);
+}
+
 static int update_info_refs(int force)
 {
-	char *path0 = git_pathdup("info/refs");
+	char *path = git_pathdup("info/refs");
-	int len = strlen(path0);
+	int ret = update_info_file(path, generate_info_refs);
-	char *path1 = xmalloc(len + 2);
+	free(path);
-
+	return ret;
-	strcpy(path1, path0);
-	strcpy(path1 + len, "+");
-
-	safe_create_leading_directories(path0);
-	info_ref_fp = fopen(path1, "w");
-	if (!info_ref_fp)
-		return error("unable to update %s", path1);
-	for_each_ref(add_info_ref, NULL);
-	fclose(info_ref_fp);
-	adjust_shared_perm(path1);
-	rename(path1, path0);
-	free(path0);
-	free(path1);
-	return 0;
 }
 
 /* packs */
@@ -198,36 +233,36 @@ static void init_pack_info(const char *infofile, int force)
 		info[i]->new_num = i;
 }
 
-static void write_pack_info_file(FILE *fp)
+static void free_pack_info(void)
 {
 	int i;
 	for (i = 0; i < num_pack; i++)
-		fprintf(fp, "P %s\n", info[i]->p->pack_name + objdirlen + 6);
+		free(info[i]);
-	fputc('\n', fp);
+	free(info);
 }
 
-static int update_info_packs(int force)
+static int write_pack_info_file(FILE *fp)
 {
-	char infofile[PATH_MAX];
+	int i;
-	char name[PATH_MAX];
+	for (i = 0; i < num_pack; i++) {
-	int namelen;
+		if (fprintf(fp, "P %s\n", info[i]->p->pack_name + objdirlen + 6) < 0)
-	FILE *fp;
+			return -1;
+	}
+	if (fputc('\n', fp) == EOF)
+		return -1;
+	return 0;
+}
 
-	namelen = sprintf(infofile, "%s/info/packs", get_object_directory());
+static int update_info_packs(int force)
-	strcpy(name, infofile);
+{
-	strcpy(name + namelen, "+");
+	char *infofile = mkpathdup("%s/info/packs", get_object_directory());
+	int ret;
 
 	init_pack_info(infofile, force);
-
+	ret = update_info_file(infofile, write_pack_info_file);
-	safe_create_leading_directories(name);
+	free_pack_info();
-	fp = fopen(name, "w");
+	free(infofile);
-	if (!fp)
+	return ret;
-		return error("cannot open %s", name);
-	write_pack_info_file(fp);
-	fclose(fp);
-	adjust_shared_perm(name);
-	rename(name, infofile);
-	return 0;
 }
 
 /* public */