Re: [PATCH] strbuf_read_file(): preserve errno across close() call

[Jeff King]

On Fri, Feb 23, 2018 at 10:00:24PM +0100, René Scharfe wrote:

> 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