From: Lars Hjemli Date: Sat, 07 Nov 2009 17:15:34 GMT Subject: Re: [cgit PATCH] Close file descriptor on error in readfile() Message-ID: <8c5c35580911070915j7a0100f5sc666b3294bca3941@mail.gmail.com> In-Reply-To: On Sat, Nov 7, 2009 at 17:14, Andreas Schwab wrote: > Lars Hjemli writes: > >> diff --git a/shared.c b/shared.c >> index d7b2d5a..a27ab30 100644 >> --- a/shared.c >> +++ b/shared.c >> @@ -406,12 +406,17 @@ int readfile(const char *path, char **buf, size_t *size) >>         fd = open(path, O_RDONLY); >>         if (fd == -1) >>                 return errno; >> -       if (fstat(fd, &st)) >> +       if (fstat(fd, &st)) { >> +               close(fd); >>                 return errno; > > The close call can clobber errno. > >> -       if (!S_ISREG(st.st_mode)) >> +       } >> +       if (!S_ISREG(st.st_mode)) { >> +               close(fd); >>                 return EISDIR; >> +       } >>         *buf = xmalloc(st.st_size + 1); >>         *size = read_in_full(fd, *buf, st.st_size); >>         (*buf)[*size] = '\0'; >> +       close(fd); >>         return (*size == st.st_size ? 0 : errno); > > Likewise. Thanks for noticing. I've applied the following patch on top of the bad one: From 21f67e7d82986135922aece6b4ebf410a98705bc Mon Sep 17 00:00:00 2001 From: Lars Hjemli Date: Sat, 7 Nov 2009 18:08:30 +0100 Subject: [PATCH] shared.c: return original errno Noticed-by: Andreas Schwab Signed-off-by: Lars Hjemli --- shared.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/shared.c b/shared.c index a27ab30..9362d21 100644 --- a/shared.c +++ b/shared.c @@ -400,15 +400,16 @@ int cgit_close_filter(struct cgit_filter *filter) */ int readfile(const char *path, char **buf, size_t *size) { - int fd; + int fd, e; struct stat st; fd = open(path, O_RDONLY); if (fd == -1) return errno; if (fstat(fd, &st)) { + e = errno; close(fd); - return errno; + return e; } if (!S_ISREG(st.st_mode)) { close(fd); @@ -416,7 +417,8 @@ int readfile(const char *path, char **buf, size_t *size) } *buf = xmalloc(st.st_size + 1); *size = read_in_full(fd, *buf, st.st_size); + e = errno; (*buf)[*size] = '\0'; close(fd); - return (*size == st.st_size ? 0 : errno); + return (*size == st.st_size ? 0 : e); } -- larsh