Web lists-archives.com

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




On Thu, Mar 16, 2017 at 11:33:36AM -0700, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> > +	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