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 <peff@xxxxxxxx>
> ---
> 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;
}
