Re: [PATCH] refs.c: add a function to append a reflog entry to a fd

Jonathan Nieder [off-list ref] writes:

quoted

Break out the code to create the string and writing it to the file
descriptor from log_ref_write and add it into a dedicated function
log_ref_write_fd.

(grammar) I'm having trouble parsing the above.

Yeah, I can see what it wants to say, but still...

quoted

Let's discuss the change and decide if I shouldsend a follow up patch to change
it into snprintf.

Both sprintf and snprintf are error-prone functions.  It would be
lovely in a followup to use strbuf_addf or xstrfmt in this code path.
strbufs are how git deals with bookkeeping for string sizes --- they
are very pleasant.

Yeah, that solves both sides of not stomping on other peoples' data
(which may lead to replaced return address and such) and not giving
broken result (which may cause trouble to the callers) equally well
without raising a stink about "security!!!", which is good ;-).

quoted

@@ -3010,19 +3032,9 @@ static int log_ref_write(const char *refname,

const unsigned char *old_sha1,

[...]

quoted

+	result = log_ref_write_fd(logfd, old_sha1, new_sha1,
+				  git_committer_info(0), msg);
+	if (result) {
 		int save_errno = errno;
 		close(logfd);
 		error("Unable to append to %s", log_file);

Since 'result' isn't used here, this could be simplified to

	if (log_ref_write_fd(...)) {
		...
	}

Yeah that is a good simplification.