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

[René Scharfe]

Am 23.02.2018 um 08:00 schrieb Jeff King:
> On Fri, Feb 23, 2018 at 01:49:52AM -0500, Jeff King wrote:
> Subject: [PATCH] strbuf_read_file(): preserve errno across close() call
> 
> If we encounter a read error, the user may want to report it
> by looking at errno. However, our close() call may clobber
> errno, leading to confusing results. Let's save and restore
> it in the error case.

Good idea.

> Signed-off-by: Jeff King <p...@peff.net>
> ---
>   strbuf.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/strbuf.c b/strbuf.c
> index 1df674e919..5f138ed3c8 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -612,14 +612,18 @@ ssize_t strbuf_read_file(struct strbuf *sb, const char 
> *path, size_t hint)
>   {
>       int fd;
>       ssize_t len;
> +     int saved_errno;
>   
>       fd = open(path, O_RDONLY);
>       if (fd < 0)
>               return -1;
>       len = strbuf_read(sb, fd, hint);
> +     saved_errno = errno;
>       close(fd);
> -     if (len < 0)
> +     if (len < 0) {
> +             errno = saved_errno;
>               return -1;
> +     }
>   
>       return len;
>   }

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.)

--- 
 strbuf.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index 1df674e919..c0066b1db9 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -2,6 +2,8 @@
 #include "refs.h"
 #include "utf8.h"
 
+#define IGNORE_ERROR(expr) do { int e_ = errno; expr; errno = e_; } while (0)
+
 int starts_with(const char *str, const char *prefix)
 {
        for (; ; str++, prefix++)
@@ -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;
@@ -617,9 +619,11 @@ ssize_t strbuf_read_file(struct strbuf *sb, const char 
*path, size_t hint)
        if (fd < 0)
                return -1;
        len = strbuf_read(sb, fd, hint);
-       close(fd);
-       if (len < 0)
+       if (len < 0) {
+               IGNORE_ERROR(close(fd));
                return -1;
+       }
+       close(fd);
 
        return len;
 }