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

Remove unused CMake variables #3166

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@ruslo
Contributor

ruslo commented Oct 24, 2018

Remove variables:

  • HAVE_SOCKLEN_T
  • CURL_SIZEOF_CURL_SOCKLEN_T
  • CURL_TYPEOF_CURL_SOCKLEN_T
Remove unused CMake variables
Remove variables:
* HAVE_SOCKLEN_T
* CURL_SIZEOF_CURL_SOCKLEN_T
* CURL_TYPEOF_CURL_SOCKLEN_T
@jzakrzewski

This comment has been minimized.

Contributor

jzakrzewski commented Oct 24, 2018

If those are really unused, aren't the checks in autotools also redundant?

@ruslo

This comment has been minimized.

Contributor

ruslo commented Oct 24, 2018

aren't the checks in autotools also redundant?

You mean this part:

  • curl/acinclude.m4

    Lines 1898 to 1899 in 4f2541f

    CURL_DEFINE_UNQUOTED([CURL_TYPEOF_CURL_SOCKLEN_T], [$curl_typeof_curl_socklen_t])
    CURL_DEFINE_UNQUOTED([CURL_SIZEOF_CURL_SOCKLEN_T], [$curl_sizeof_curl_socklen_t])

?

CURL_SIZEOF_CURL_SOCKLEN_T can be removed indeed. I think it can be done as a follow-up by a person who use autotools to build CURL and have some experience with the language, because some related code should be removed too.

I'm not so sure about CURL_TYPEOF_CURL_SOCKLEN_T. CURL_TYPEOF_CURL_SOCKLEN_T defined at compile time for almost all cases except few, so probably the value of CURL_TYPEOF_CURL_SOCKLEN_T defined by autotools sometimes used (?):

Anyway CURL_TYPEOF_CURL_SOCKLEN_T is not used in CMake part because it's not defined here:

@bagder

This comment has been minimized.

Member

bagder commented Oct 24, 2018

CURL_TYPEOF_CURL_SOCKLEN_T

This is defined by the public header curl/system.h so that should be totally fine to not define separately.

I can take care of the autotools cleanup of the *SOCKLEN_T macros.

@jzakrzewski

This comment has been minimized.

Contributor

jzakrzewski commented Oct 24, 2018

Yeah, I meant that.
It would be quite surprising if one build system needed to define those and other didn't.

bagder added a commit that referenced this pull request Oct 25, 2018

configure: remove CURL_CONFIGURE_CURL_SOCKLEN_T
Follow-up to #3166 which did the cmake part of this. This type/define is
not used.

@bagder bagder closed this in b04e624 Oct 25, 2018

bagder added a commit that referenced this pull request Oct 25, 2018

configure: remove CURL_CONFIGURE_CURL_SOCKLEN_T
Follow-up to #3166 which did the cmake part of this. This type/define is
not used.

Closes #3168

@ruslo ruslo deleted the ruslo:pr.unused branch Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment