Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tool_operate: typecheck-gcc warning #1403

Closed
MarcelRaad opened this issue Apr 10, 2017 · 5 comments
Closed

tool_operate: typecheck-gcc warning #1403

MarcelRaad opened this issue Apr 10, 2017 · 5 comments

Comments

@MarcelRaad
Copy link
Member

MarcelRaad commented Apr 10, 2017

I did this

Compiled with --disable-libcurl-option --enable-werror.

With MinGW / GCC 6.3.0, I got:

In file included from tool_operate.c:70:0:
tool_operate.c: In function 'operate_do':
../include/curl/typecheck-gcc.h:83:9: error: call to '_curl_easy_setopt_err_seek_cb' declared with attribute warning: curl_easy_setopt expects a curl_seek_callback argument for this option [-Werror]
         _curl_easy_setopt_err_seek_cb();                                      \
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tool_setopt.h:31:13: note: in definition of macro 'SETOPT_CHECK'
   result = (v); \
             ^
tool_setopt.h:126:16: note: in expansion of macro 'curl_easy_setopt'
   SETOPT_CHECK(curl_easy_setopt(x, y, z))
                ^~~~~~~~~~~~~~~~
tool_operate.c:889:9: note: in expansion of macro 'my_setopt'
         my_setopt(curl, CURLOPT_SEEKFUNCTION, tool_seek_cb);
         ^~~~~~~~~

Passing a function pointer to my_setopt fixes the issue (but all the other my_setopt calls, which compile successfully, don't do that either):
my_setopt(curl, CURLOPT_SEEKFUNCTION, &tool_seek_cb);

I also noticed that _curl_conv_callback* and _curl_seek_callback* are defined as pointers in typecheck-gcc.h, but all other function types aren't. Removing the * from them works around the issue too (and should be done because of _curl_callback_compatible checking for a pointer to the argument in addition to the argument itself), but choosing one of the alternatives returning CURLcode is probably not what is intended when the function declaration exactly matches the return type and parameters of curl_seek_callback, also returning an int.

Probably every __builtin_types_compatible_p(__typeof__(expr), curl_*_callback) in _curl_is_*_cb should also have a __builtin_types_compatible_p(__typeof__(expr) *, curl_*_callback)?

(I'll also prepare a pull request.)

I expected the following

Successful compilation without warnings.

curl/libcurl version

git master f9d1e9a

operating system

Windows / MSYS2

@bagder
Copy link
Member

bagder commented Apr 10, 2017

Ah, I suppose I need to build without --enable-debug or something to trigger that error so I haven't detected this before! What a weird error, the seek callback looks like it is set and used like the other callbacks...

@MarcelRaad
Copy link
Member Author

MarcelRaad commented Apr 10, 2017

Ah, I suppose I need to build without --enable-debug or something to trigger that error so I haven't detected this before!

I think the key is --disable-libcurl-option, which makes curl call curl_easy_setopt (defined to that typecheck macro) directly instead of a wrapper function.

@bagder
Copy link
Member

bagder commented Apr 10, 2017

Oh yes, that's it.

MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Apr 10, 2017
All the callbacks passed to curl_easy_setopt are defined as function
pointers. The possibility to pass both functions and function pointers
was handled for the callbacks that typecheck-gcc.h defined as
compatible, but not for the public callback types themselves.

This makes all compatible callback types defined in typecheck-gcc.h
function pointers too and checks all functions uniformly with
_curl_callback_compatible, which handles both functions and function
pointers.

A symptom of the problem was a warning in tool_operate.c with
--disable-libcurl-option as that file passes the callback functions
to curl_easy_setopt directly.

Fixes curl#1403
@bagder
Copy link
Member

bagder commented Apr 10, 2017

We should write a libcurl test that actually uses every single libcurl option, so that we can see that typecheck-gcc at least works for the valid sets...

@MarcelRaad
Copy link
Member Author

Absolutely. I'll try, but it will take some days.

So it has to be in the "tests/unit" directory, right? And I guess there are no compile-time tests yet, are there?

MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Apr 12, 2017
All the callbacks passed to curl_easy_setopt are defined as function
pointers. The possibility to pass both functions and function pointers
was handled for the callbacks that typecheck-gcc.h defined as
compatible, but not for the public callback types themselves.

This makes all compatible callback types defined in typecheck-gcc.h
function pointers too and checks all functions uniformly with
_curl_callback_compatible, which handles both functions and function
pointers.

A symptom of the problem was a warning in tool_operate.c with
--disable-libcurl-option as that file passes the callback functions
to curl_easy_setopt directly.

Fixes curl#1403
MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Apr 17, 2017
All the callbacks passed to curl_easy_setopt are defined as function
pointers. The possibility to pass both functions and function pointers
was handled for the callbacks that typecheck-gcc.h defined as
compatible, but not for the public callback types themselves.

This makes all compatible callback types defined in typecheck-gcc.h
function pointers too and checks all functions uniformly with
_curl_callback_compatible, which handles both functions and function
pointers.

A symptom of the problem was a warning in tool_operate.c with
--disable-libcurl-option as that file passes the callback functions
to curl_easy_setopt directly.

Fixes curl#1403
MarcelRaad added a commit to MarcelRaad/curl that referenced this issue Apr 21, 2017
All the callbacks passed to curl_easy_setopt are defined as function
pointers. The possibility to pass both functions and function pointers
was handled for the callbacks that typecheck-gcc.h defined as
compatible, but not for the public callback types themselves.

This makes all compatible callback types defined in typecheck-gcc.h
function pointers too and checks all functions uniformly with
_curl_callback_compatible, which handles both functions and function
pointers.

A symptom of the problem was a warning in tool_operate.c with
--disable-libcurl-option and without --enable-debug as that file
passes the callback functions to curl_easy_setopt directly.

Fixes curl#1403
Closes curl#1404
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants