Re: [PATCH] strbuf_read_file(): preserve errno across close() call
From: Jeff King <hidden>
Date: 2018-02-26 09:04:42
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Fri, Feb 23, 2018 at 10:00:24PM +0100, René Scharfe wrote:
quoted hunk ↗ jump to hunk
How about adding a stealthy close_no_errno(), or do something like the following to get shorter and more readable code? (We could also keep a single close() call, but would then set errno even on success.) [...]@@ -391,7 +393,7 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint) if (got < 0) { if (oldalloc == 0) - strbuf_release(sb); + IGNORE_ERROR(strbuf_release(sb)); else strbuf_setlen(sb, oldlen); return -1;
I dunno, that may be crossing the line of "too magical". I had envisioned something like:
diff --git a/strbuf.c b/strbuf.c
index 5f138ed3c8..0790dd7bcb 100644
--- a/strbuf.c
+++ b/strbuf.c@@ -365,6 +365,14 @@ void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src) } } +/* release, but preserve errno */ +static void strbuf_release_careful(struct strbuf *sb) +{ + int saved_errno = errno; + strbuf_release(sb); + errno = saved_errno; +} + size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f) { size_t res;
@@ -375,7 +383,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f) if (res > 0) strbuf_setlen(sb, sb->len + res); else if (oldalloc == 0) - strbuf_release(sb); + strbuf_release_careful(sb); return res; }
@@ -391,7 +399,7 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint) if (got < 0) { if (oldalloc == 0) - strbuf_release(sb); + strbuf_release_careful(sb); else strbuf_setlen(sb, oldlen); return -1;
@@ -416,7 +424,7 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint) if (cnt > 0) strbuf_setlen(sb, sb->len + cnt); else if (oldalloc == 0) - strbuf_release(sb); + strbuf_release_careful(sb); return cnt; }
@@ -482,7 +490,7 @@ int strbuf_getcwd(struct strbuf *sb) break; } if (oldalloc == 0) - strbuf_release(sb); + strbuf_release_careful(sb); else strbuf_reset(sb); return -1;
but that solution is definitely very specific to these cases. I also had a feeling I should be able to shove the "oldalloc" logic into the helper, too, but there are too many different behaviors in the "else" block. -Peff