On Fri, Feb 23, 2018 at 01:49:52AM -0500, Jeff King wrote:
> > +static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path)
> > +{
> > + int fd;
> > + ssize_t len;
> > +
> > + fd = open(path, O_RDONLY);
> > + if (fd < 0)
> > + return error_errno(_("could not open '%s'"), path);
> > + len = strbuf_read(sb, fd, 0);
> > + close(fd);
> > + if (len < 0)
> > + return error(_("could not read '%s'."), path);
> > + return len;
> > +}
>
> If we were to use error_errno() in the second conditional here, we
> should take care not to clobber errno during the close(). I think
> strbuf_read_file() actually has the same problem, which might be worth
> fixing.
Here's a patch, while I'm thinking about it.
I notice that quite a few strbuf error paths may call strbuf_release(),
too. Technically free() may clobber errno, too. I don't know if it's
worth protecting against (IIRC POSIX is being amended to disallow this,
but I have no idea how common it is in existing platforms).
-- >8 --
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.
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;
}
--
2.16.2.580.g96c83ce8ea
