Re: [PATCH v2 4/5] replace snprintf with odb_pack_name()

2 messages, 2 authors, 2017-03-16 · open the first message on its own page

Re: [PATCH v2 4/5] replace snprintf with odb_pack_name()

From: Junio C Hamano <hidden>
Date: 2017-03-16 18:33:50

Jeff King [off-list ref] writes:
quoted hunk
+	struct strbuf name = STRBUF_INIT;
 	int err;
 
 	if (!from_stdin) {
@@ -1402,14 +1402,13 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		int keep_fd, keep_msg_len = strlen(keep_msg);
 
 		if (!keep_name)
-			snprintf(name, sizeof(name), "%s/pack/pack-%s.keep",
-				 get_object_directory(), sha1_to_hex(sha1));
+			odb_pack_name(&name, sha1, "keep");
 
-		keep_fd = odb_pack_keep(keep_name ? keep_name : name);
+		keep_fd = odb_pack_keep(keep_name ? keep_name : name.buf);
 		if (keep_fd < 0) {
 			if (errno != EEXIST)
 				die_errno(_("cannot write keep file '%s'"),
-					  keep_name ? keep_name : name);
+					  keep_name ? keep_name : name.buf);
 		} else {
 			if (keep_msg_len > 0) {
 				write_or_die(keep_fd, keep_msg, keep_msg_len);
@@ -1417,28 +1416,22 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 			}
 			if (close(keep_fd) != 0)
 				die_errno(_("cannot close written keep file '%s'"),
-					  keep_name ? keep_name : name);
+					  keep_name ? keep_name : name.buf);
 			report = "keep";
 		}
 	}

The patch as-posted is obviously with less damage to the current
code, but the above makes me wonder if it makes simpler to do

	if (keep_name)
		strbuf_addstr(&name, keep_name);
	else
		odb_pack_name(&name, sha1, "keep");

so that we can always use name.buf without having to do "?:" thing.

Re: [PATCH v2 4/5] replace snprintf with odb_pack_name()

From: Jeff King <hidden>
Date: 2017-03-16 18:35:00

On Thu, Mar 16, 2017 at 11:33:36AM -0700, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
+	struct strbuf name = STRBUF_INIT;
 	int err;
 
 	if (!from_stdin) {
@@ -1402,14 +1402,13 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 		int keep_fd, keep_msg_len = strlen(keep_msg);
 
 		if (!keep_name)
-			snprintf(name, sizeof(name), "%s/pack/pack-%s.keep",
-				 get_object_directory(), sha1_to_hex(sha1));
+			odb_pack_name(&name, sha1, "keep");
 
-		keep_fd = odb_pack_keep(keep_name ? keep_name : name);
+		keep_fd = odb_pack_keep(keep_name ? keep_name : name.buf);
 		if (keep_fd < 0) {
 			if (errno != EEXIST)
 				die_errno(_("cannot write keep file '%s'"),
-					  keep_name ? keep_name : name);
+					  keep_name ? keep_name : name.buf);
 		} else {
 			if (keep_msg_len > 0) {
 				write_or_die(keep_fd, keep_msg, keep_msg_len);
@@ -1417,28 +1416,22 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 			}
 			if (close(keep_fd) != 0)
 				die_errno(_("cannot close written keep file '%s'"),
-					  keep_name ? keep_name : name);
+					  keep_name ? keep_name : name.buf);
 			report = "keep";
 		}
 	}

The patch as-posted is obviously with less damage to the current
code, but the above makes me wonder if it makes simpler to do

	if (keep_name)
		strbuf_addstr(&name, keep_name);
	else
		odb_pack_name(&name, sha1, "keep");

so that we can always use name.buf without having to do "?:" thing.
See the next patch. :)

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help