Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

Use correct parameter types when calling curl_easy_setopt() #61

Merged
merged 1 commit into from
Jun 4, 2017

Conversation

mkauf
Copy link
Contributor

@mkauf mkauf commented Jun 2, 2017

Fix compiler warnings, including this one:

In file included from /home/test/curl/include/curl/curl.h:2515:0,
                 from geoipupdate.h:6,
                 from geoipupdate.c:2:
geoipupdate.c: In function 'download_to_file':
/home/test/curl/include/curl/typecheck-gcc.h:56:9: warning: call to '_curl_easy_setopt_err_write_callback' declared with attribute warning: curl_easy_setopt expects a curl_write_callback argument for this option
         _curl_easy_setopt_err_write_callback();                               \
         ^
geoipupdate.c:538:5: note: in expansion of macro 'curl_easy_setopt'
     curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, get_expected_file_md5);
     ^

@horgh
Copy link
Contributor

horgh commented Jun 2, 2017

Hi @mkauf. Thanks very much for the contribution!

The changes look good.

I am curious about how you saw these warnings. I'd like to ensure we see any similar ones going forward! Are you able to tell me anything about the OS/compiler version/curl version you're using, and any flags you used to enable them?

@mkauf
Copy link
Contributor Author

mkauf commented Jun 4, 2017

I have used a recent curl version (7.54.0). Probably old curl versions do not trigger this warning.

You also need to compile with GCC 4.3 or later to get this warning, see https://github.com/curl/curl/blob/master/include/curl/curl.h :

#if defined(__GNUC__) && defined(__GNUC_MINOR__) && \
    ((__GNUC__ > 4) || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)) && \
    !defined(__cplusplus) && !defined(CURL_DISABLE_TYPECHECK)
#include "typecheck-gcc.h"
#else
...

@horgh
Copy link
Contributor

horgh commented Jun 4, 2017

Interesting! Thanks for describing that.

I found that I needed to pass -Wsystem-headers to gcc in order to see the warning in my environment.

As far as being able to see such warnings in the future, I don't think adding -Wsystem-headers to the default CFLAGS would be appropriate for us. I tried adding the flag for our CI builds, but it unfortunately does not raise this warning. I'm wondering if it is due to different gcc/libcurl versions.

Anyway, your change looks great! Thank you again!

@horgh horgh merged commit 8e48bb1 into maxmind:master Jun 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants