Re: [cgit PATCH] Close file descriptor on error in readfile()

[Lars Hjemli <hjemli@xxxxxxxxx>]

On Sat, Nov 7, 2009 at 17:14, Andreas Schwab <schwab@xxxxxxxxxxxxxx> wrote:
> Lars Hjemli <hjemli@xxxxxxxxx> 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 <hjemli@xxxxxxxxx>
Date: Sat, 7 Nov 2009 18:08:30 +0100
Subject: [PATCH] shared.c: return original errno

Noticed-by: Andreas Schwab <schwab@xxxxxxxxxxxxxx>
Signed-off-by: Lars Hjemli <hjemli@xxxxxxxxx>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html