Re: [PATCH v2 21/25] sequencer: refactor write_message()
From: Junio C Hamano <hidden>
Date: 2016-09-11 23:38:16
Johannes Schindelin [off-list ref] writes:
The write_message() function safely writes an strbuf to a file. Sometimes it is inconvenient to require an strbuf, though: the text to be written may not be stored in a strbuf, or the strbuf should not be released after writing. Let's refactor "safely writing string to a file" into write_with_lock_file(), and make write_message() use it. The new function makes it easy to create new convenience function write_file_gently(); as some of the upcoming callers of this new function would want to append a newline character, add a flag for it in write_file_gently(), and thus in write_with_lock_file().
Makes sense. As an abstraction, "give me strbuf for my sole use, and I'll trash its contents when I am done" feels like a horrible helper interface; perhaps that was overly aggressively refactored by noticing that then-current callers all released the strbuf, but still feels wrong. And this makes the underlying helper a lot more useful.
quoted hunk
Signed-off-by: Johannes Schindelin <redacted> --- sequencer.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)diff --git a/sequencer.c b/sequencer.c index 5e5d113..aa949d4 100644 --- a/sequencer.c +++ b/sequencer.c@@ -242,22 +242,37 @@ static void print_advice(int show_hint, struct replay_opts *opts) } } -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; } +static int write_message(struct strbuf *msgbuf, const char *filename) +{ + int res = write_with_lock_file(filename, msgbuf->buf, msgbuf->len, 0); + strbuf_release(msgbuf); + return res; +} + +static int write_file_gently(const char *filename, + const char *text, int append_eol) +{ + return write_with_lock_file(filename, text, strlen(text), append_eol); +} + /* * Reads a file that was presumably written by a shell script, i.e. * with an end-of-line marker that needs to be stripped.