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

Comments

Projects
None yet
2 participants
@MarcelRaad
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

This comment has been minimized.

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

This comment has been minimized.

Member

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

This comment has been minimized.

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

typecheck-gcc: handle function pointers properly
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

This comment has been minimized.

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

This comment has been minimized.

Member

MarcelRaad commented Apr 11, 2017

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

typecheck-gcc: handle function pointers properly
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

typecheck-gcc: handle function pointers properly
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

typecheck-gcc: handle function pointers properly
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.