m6w6 / ext-http

Extended HTTP Support
BSD 2-Clause "Simplified" License
79 stars 22 forks source link

Fix some build warnings #124

Closed remicollet closed 2 years ago

remicollet commented 2 years ago

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 commented 2 years ago

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[bot] commented 2 years ago

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 commented 2 years ago
/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: https://github.com/m6w6/ext-http/blob/4ca2476be7948adeaf9bca2f40d53ff9c12ebc28/src/php_http_querystring.c#L253

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.