# [PATCH] Check the return value of do_fstat() to avoid error conditions. - Thread: https://externals.io/message/80520 - Original archive: https://news-web.php.net/php.internals/80520 - Started by Joshua Rogers on 2015-01-15T08:04:47+00:00 - Messages: 6 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #80520 — Joshua Rogers — 2015-01-15T08:04:47+00:00 URL: https://externals.io/message/80520#80520 From: Joshua Rogers --- 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 --- ## #81186 — Yasuo Ohgaki — 2015-01-27T00:35:57+00:00 URL: https://externals.io/message/80520#81186 · In reply to: #80520 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 --- ## #81187 — Joshua Rogers — 2015-01-27T00:39:28+00:00 URL: https://externals.io/message/80520#81187 · In reply to: #81186 > 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 > […] > 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 --- ## #81195 — Joshua Rogers — 2015-01-27T03:32:09+00:00 URL: https://externals.io/message/80520#81195 · In reply to: #81187 > 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 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