[PATCH] Check the return value of do_fstat() to avoid error conditions.

9 years ago by Joshua Rogers — view source

unread

main/streams/plain_wrapper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/main/streams/plain_wrapper.c b/main/streams/plain_wrapper.c
index f52383d..72be0b0 100644
--- a/main/streams/plain_wrapper.c
+++ b/main/streams/plain_wrapper.c
@@ -669,7 +669,8 @@ static int php_stdiop_set_option(php_stream *stream, int option, int value, void
return fd == -1 ? PHP_STREAM_OPTION_RETURN_ERR : PHP_STREAM_OPTION_RETURN_OK;

				case PHP_STREAM_MMAP_MAP_RANGE:

```
  				do_fstat(data, 1);
```

```
  				if(do_fstat(data, 1)<0)
```

  					return -1;
  				if (range->length == 0 && range->offset > 0 && range->offset < data->sb.st_size) {
  					range->length = data->sb.st_size - range->offset;
  				}

--
1.9.1

9 years ago by Yasuo Ohgaki — view source

unread

Hi all,

main/streams/plain_wrapper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/main/streams/plain_wrapper.c b/main/streams/plain_wrapper.c
index f52383d..72be0b0 100644
--- a/main/streams/plain_wrapper.c
+++ b/main/streams/plain_wrapper.c
@@ -669,7 +669,8 @@ static int php_stdiop_set_option(php_stream *stream,
int option, int value, void
return fd == -1 ?
PHP_STREAM_OPTION_RETURN_ERR : PHP_STREAM_OPTION_RETURN_OK;
                                    case PHP_STREAM_MMAP_MAP_RANGE:
                                          do_fstat(data, 1);
                                          if(do_fstat(data, 1)<0)
                                                  return -1;
                                          if (range->length == 0 &&
range->offset > 0 && range->offset < data->sb.st_size) {
range->length =
data->sb.st_size - range->offset;
}

This patch seems reasonable. Does anyone taking care of this?

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Joshua Rogers — view source

unread

This patch seems reasonable. Does anyone taking care of this?
Perhaps it should be

if(do_fstat(data, 1)<0) {
return -1;
}

just to conform with the rest of the code

--
-- Joshua Rogers <https://internot.info/

9 years ago by Yasuo Ohgaki — view source

unread

Hi Joshua,

This patch seems reasonable. Does anyone taking care of this?
Perhaps it should be

if(do_fstat(data, 1)<0) {
return -1;
}

just to conform with the rest of the code

If nobody is taking care of this yet, please send PR or bug report.
Thank you.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

9 years ago by Joshua Rogers — view source

unread

if(do_fstat(data, 1)<0) {
return -1;
}
This was wrong, anyways.

if '1' is returned, the it is OK to continue.
If '0' is returned, it is not OK to continue(return -1)

Since this is the case, should the do_fstat function be changed to a
boolean?

It is only possible for it to return 0 and 1:

           d->cached_fstat = r == 0;

            return r;
    }
    return 0;

It makes it confusing when reading the code that it is defined as an
int, but the output can only be 0/1.

Does anybody object to me changing to from a 'static int', to a 'static
zend_bool'?

I'll submit a PR, but I would be interested in feedback via the mailing
list.

Thanks,

--
-- Joshua Rogers <https://internot.info/

9 years ago by Joshua Rogers — view source

unread

if '1' is returned, the it is OK to continue.
If '0' is returned, it is not OK to continue(return -1)

Since this is the case, should the do_fstat function be changed to a
boolean?
https://github.com/php/php-src/pull/1031

--
-- Joshua Rogers <https://internot.info/