Stefan Beller wrote: > From: Ronnie Sahlberg <sahlberg@xxxxxxxxxx> > > Move code to create the string for a ref and write it to a file descriptor > from log_ref_write and add it into a new dedicated function > log_ref_write_fd. > > For now the new function is only used from log_ref_write, but later > on we will call this function from reflog transactions too. That means > that we will end up with only a single place, where we write a > reflog entry to a file instead of the current two places > (log_ref_write and builtin/reflog.c). Line-wrapping width is still inconsistent. I don't think it's worth resending just for that, but something to look out for in the future. > Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx> > Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> [...] > +++ b/refs.c [...] > @@ -3010,19 +3032,9 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1, [...] > + result = log_ref_write_fd(logfd, old_sha1, new_sha1, > + git_committer_info(0), msg); > + if (result) { > int save_errno = errno; I don't understand why the above writes to a temporary variable and checks it, never to read that temporary again. I don't think that alone is a reason to block the patch, but it worries me in that the review comment seems to have been just lost. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html