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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 16, 2017, 18:33 UTC
Message-ID: <xmqqvar9ax6n.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170316142715.zdoikgyy3difeafk@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 31 quoted lines

> +	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.