git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 6/8] write_file: add pointer+len variant

From
Jeff King <peff@peff.net>
Date
Jul 8, 2016, 09:12 UTC
Message-ID
<20160708091222.GF10152@sigill.intra.peff.net>
In-Reply-To
<20160708090400.GA26594@sigill.intra.peff.net>

There are many callsites which could use write_file, but for which it is a little awkward because they have a strbuf or other pointer/len combo. Specifically:

 1. write_file() takes a format string, so we have to use
    "%s" or "%.*s", which are ugly.
 2. Using any form of "%s" does not handle embedded NULs in
    the output. That probably doesn't matter for our
    call-sites, but it's nicer not to have to worry.
 3. It's less efficient; we format into another strbuf
    just to do the write. That's probably not measurably
    slow for our uses, but it's simply inelegant.

We can fix this by providing a helper to write out the formatted buffer, and just calling it from write_file().

Note that we don't do the usual "complete with a newline" that write_file does. If the caller has their own buffer, there's a reasonable chance they're doing something more complicated than a single line, and they can call strbuf_complete_line() themselves.

We could go even further and add strbuf_write_file(), but it doesn't save much:

  -  write_file_buf(path, sb.buf, sb.len);
  +  strbuf_write_file(&sb, path);

It would also be somewhat asymmetric with strbuf_read_file, which actually returns errors rather than dying (and the error handling is most of the benefit of write_file() in the first place).

Signed-off-by: Jeff King <peff@peff.net>
---
 cache.h   |  6 ++++++
 wrapper.c | 16 +++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index 3f6c53f..9d6ad4f 100644
--- a/cache.h
+++ b/cache.h
@@ -1745,6 +1745,12 @@ static inline ssize_t write_str_in_full(int fd, const char *str)
 	return write_in_full(fd, str, strlen(str));
 }
 
+/**
+ * Open (and truncate) the file at path, write the contents of buf to it,
+ * and close it. Dies if any errors are encountered.
+ */
+extern void write_file_buf(const char *path, const char *buf, size_t len);
+
 extern void write_file(const char *path, const char *fmt, ...);
 
 /* pager.c */
diff --git a/wrapper.c b/wrapper.c
index 7c126b8..b827206 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -640,22 +640,28 @@ int xsnprintf(char *dst, size_t max, const char *fmt, ...)
 	return len;
 }
 
+void write_file_buf(const char *path, const char *buf, size_t len)
+{
+	int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+	if (write_in_full(fd, buf, len) != len)
+		die_errno(_("could not write to %s"), path);
+	if (close(fd))
+		die_errno(_("could not close %s"), path);
+}
+
 void write_file(const char *path, const char *fmt, ...)
 {
 	va_list params;
 	struct strbuf sb = STRBUF_INIT;
-	int fd = xopen(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
 
 	va_start(params, fmt);
 	strbuf_vaddf(&sb, fmt, params);
 	va_end(params);
 
 	strbuf_complete_line(&sb);
-	if (write_in_full(fd, sb.buf, sb.len) != sb.len)
-		die_errno(_("could not write to %s"), path);
+
+	write_file_buf(path, sb.buf, sb.len);
 	strbuf_release(&sb);
-	if (close(fd))
-		die_errno(_("could not close %s"), path);
 }
 
 void sleep_millisec(int millisec)
-- 
2.9.0.393.g704e522
Previous: Jeff KingNext: Jeff King
Message 11 of 27 in “am: ignore return value of write_file()”
  1. am: ignore return value of write_file()René Scharfe, Jul 7, 2016
  2. Jeff KingJul 7, 2016
  3. Johannes SchindelinJul 8, 2016
  4. Jeff KingJul 8, 2016
  5. 0/8 write_file cleanupsJeff King, Jul 8, 2016
  6. 1/8 config: fix bogus fd check when setting up default configJeff King, Jul 8, 2016
  7. 2/8 am: ignore return value of write_file()Jeff King, Jul 8, 2016
  8. 3/8 branch: use non-gentle write_file for branch descriptionJeff King, Jul 8, 2016
  9. 4/8 write_file: drop "gently" formJeff King, Jul 8, 2016
  10. 5/8 write_file: use xopenJeff King, Jul 8, 2016
  11. 6/8 write_file: add pointer+len variantJeff King, Jul 8, 2016
  12. 7/8 write_file: add format attributeJeff King, Jul 8, 2016
  13. Jeff KingJul 8, 2016
  14. 1/2 walker: let walker_say take arbitrary formatsJeff King, Jul 8, 2016
  15. 2/2 avoid using sha1_to_hex output as printf formatJeff King, Jul 8, 2016
  16. Jeff KingJul 8, 2016
  17. Junio C HamanoJul 8, 2016
  18. Junio C HamanoJul 8, 2016
  19. Jeff KingJul 8, 2016
  20. 8/8 use write_file_buf where applicableJeff King, Jul 8, 2016
  21. 9/8 branch: use write_file_buf instead of write_fileJeff King, Jul 8, 2016
  22. René ScharfeJul 8, 2016
  23. Johannes SchindelinJul 9, 2016
  24. Johannes SchindelinJul 10, 2016
  25. Johannes SchindelinJul 8, 2016
  26. René ScharfeJul 8, 2016
  27. Jeff KingJul 8, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.