Hi Junio & Hannes, On Thu, 15 Sep 2016, Junio C Hamano wrote: > Johannes Sixt <j6t@xxxxxxxx> writes: > > > Am 11.09.2016 um 12:55 schrieb Johannes Schindelin: > >> -static int write_message(struct strbuf *msgbuf, const char *filename) > >> +static int write_with_lock_file(const char *filename, > >> + const void *buf, size_t len, int append_eol) > >> { > >> static struct lock_file msg_file; > >> > >> int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0); > >> if (msg_fd < 0) > >> return error_errno(_("Could not lock '%s'"), filename); > >> - if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0) > >> - return error_errno(_("Could not write to %s"), filename); > >> - strbuf_release(msgbuf); > >> + if (write_in_full(msg_fd, buf, len) < 0) > >> + return error_errno(_("Could not write to '%s'"), filename); > >> + if (append_eol && write(msg_fd, "\n", 1) < 0) > >> + return error_errno(_("Could not write eol to '%s"), filename); > >> if (commit_lock_file(&msg_file) < 0) > >> return error(_("Error wrapping up %s."), filename); > >> > >> return 0; > >> } > > > > The two error paths in the added lines should both > > > > rollback_lock_file(&msg_file); > > > > , I think. But I do notice that this is not exactly new, so... > > It may not be new for this step, but overall the series is aiming to > libify the stuff, so we should fix fd and lockfile leaks like this > as we notice them. Makes sense, even for the final commit_lock_file(). Ciao, Dscho