Skip to content

Conversation

@remicollet
Copy link
Collaborator

At least first commit seems a real problem
I dont know is change from int / size_t is related to some libcurl version, but at least already size_t in 7.29 on RHEL 7

@remicollet
Copy link
Collaborator Author

Remains:

In file included from /usr/include/php/Zend/zend.h:27,
                 from /usr/include/php/main/php.h:31,
                 from /work/GIT/pecl-and-ext/http/src/php_http_api.h:31,
                 from /work/GIT/pecl-and-ext/http/src/php_http_querystring.c:13:
/work/GIT/pecl-and-ext/http/src/php_http_querystring.c: In function 'php_http_querystring_update':
/usr/include/php/Zend/zend_types.h:1174:16: warning: 'zv.value.counted' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1174 |         return --(p->refcount);
      |                ^~~~~~~~~~~~~~~
/work/GIT/pecl-and-ext/http/src/php_http_querystring.c:251:22: note: 'zv.value.counted' was declared here
  251 |                 zval zv, *params_entry, *qarray_entry;
      |                      ^~

And

/work/GIT/pecl-and-ext/http/src/php_http_url.c: In function 'php_http_url_parse':
/work/GIT/pecl-and-ext/http/src/php_http_url.c:1825:25: warning: '%s' directive argument is null [-Wformat-overflow=]
 1825 |                         php_error_docref(NULL, E_WARNING, "Failed to parse URL scheme: '%s'", state->ptr);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@codecov
Copy link

codecov bot commented Feb 25, 2022

Codecov Report

Merging #124 (1e6aa1b) into master (4ca2476) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   85.39%   85.39%           
=======================================
  Files          42       42           
  Lines        9291     9291           
=======================================
  Hits         7934     7934           
  Misses       1357     1357           
Impacted Files Coverage Δ
src/php_http_message.c 87.59% <ø> (ø)
src/php_http_client_curl.c 81.40% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ca2476...1e6aa1b. Read the comment docs.

@m6w6
Copy link
Owner

m6w6 commented Feb 25, 2022

/work/GIT/pecl-and-ext/http/src/php_http_querystring.c:251:22: note: 'zv.value.counted' was declared here
  251 |                 zval zv, *params_entry, *qarray_entry;
      |                      ^~

Looks like a false positive? There's a ZVAL_NULL right below:

ZVAL_NULL(&zv);

And

/work/GIT/pecl-and-ext/http/src/php_http_url.c:1825:25: warning: '%s' directive argument is null [-Wformat-overflow=]
 1825 |                         php_error_docref(NULL, E_WARNING, "Failed to parse URL scheme: '%s'", state->ptr);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Ugh, yeah, while this function is never called with NULL, nobody did tell that to the compiler, so technically valid, I guess.

@m6w6 m6w6 merged commit 29bdc70 into master Feb 25, 2022
@remicollet remicollet deleted the issue-warn branch February 26, 2022 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants