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

configure: looking for strcasecmp in -lresolve (sic) #770

Closed
Irfy opened this issue Apr 17, 2016 · 4 comments
Closed

configure: looking for strcasecmp in -lresolve (sic) #770

Irfy opened this issue Apr 17, 2016 · 4 comments
Labels

Comments

@Irfy
Copy link

Irfy commented Apr 17, 2016

The following code is located immediately after searching for gethostbyname in configure.ac:

dnl resolve lib?
AC_CHECK_FUNC(strcasecmp, , [ AC_CHECK_LIB(resolve, strcasecmp) ])

if test "$ac_cv_lib_resolve_strcasecmp" = "$ac_cv_func_strcasecmp"; then
  AC_CHECK_LIB(resolve, strcasecmp,
              [LIBS="-lresolve $LIBS"],
               ,
               -lnsl)
fi
ac_cv_func_strcasecmp="no"

There are several issues with this code:

  • I do not know of any library named libresolve, only libresolv
  • I have never seen strcasecmp being searched for in a networking library
  • If the first AC_CHECK_LIB succeeds, -lresolve is never added to LIBS which makes no sense
  • Blindly setting the cache variable to "no" in the end is likely to wreak havoc in projects relying on the cache

I have the feeling this is some really old, untested code that was never executed, simply because AC_CHECK_FUNC re-used the cached variable ac_cv_func_strcasecmp which is set earlier, in a call to CURL_CHECK_FUNC_STRCASECMP, and which is highly likely to be "yes". (I originally confused the location of CURL_CHECK_FUNC_STRCASECMP relative to this piece of code)

When this variable is "no" AC_CHECK_FUNC fails, assuming the library name is a typo, AC_CHECK_LIB will never succeed and nothing will happen. When AC_CHECK_FUNC succeeds, the rest is never executed and nothing will happen.

The code seems to be from 1999/2000, which fits with the existence of the library libresolv.

I have the feeling, the author meant to look for gethostbyname in libresolv, as this function is searched in multiple libraries right before this piece of code.

If I'm wrong about any of this, I beg for a reference to libresolve and for an explanation about looking for strcasecmp there.

If I'm right about this, I would gladly rewrite the code to look for gethostbyname (seems to have been in libresolv in 1997)

Irfy pushed a commit to Irfy/curl that referenced this issue Apr 17, 2016
These configure vars are modified in a curl-specific way but never
evaluated or loaded from cache, even though they are designated as
_cv_. We could either implement proper AC_CACHE_CHECKs for them, or
remove them completely.

Fixes curl#603 as ac_cv_func_gethostbyname is no longer clobbered, and
AC_CHECK_FUNC(gethostbyname...) will no longer spuriously succeed after
the first configure run with caching.

`ac_cv_func_strcasecmp` is curious, see curl#770.

`eval "ac_cv_func_$func=yes"` can still cause problems as it works in
tandem with AC_CHECK_FUNCS and then potentially modifies its result. It
would be best to rewrite this test to use a new CURL_CHECK_FUNCS macro,
which works the same as AC_CHECK_FUNCS but relies on caching the values
of curl_cv_func_* variables, without modifiying ac_cv_func_*.
@bagder
Copy link
Member

bagder commented Apr 18, 2016

strncasecmp was provided by libresolv (without a trailing 'e') for sunos, and that's even still documented

I think that configure check worked at some point but the wrong lib name makes it seem unlikely to work at all like this.

I think we can just remove that check now.

@bagder bagder added the build label Apr 18, 2016
@Irfy
Copy link
Author

Irfy commented Apr 18, 2016

Should I piggyback a commit removing this code to #766?

bagder added a commit that referenced this issue Apr 18, 2016
'strncasecmp' was once provided by libresolv (no trailing e) for SunOS,
but this check is broken and most likely adds nothing useful. Removing
now.

Reported-by: Irfan Adilovic

Discussed in #770
@bagder
Copy link
Member

bagder commented Apr 18, 2016

I just pushed this change individually to master in commit fb823d2

@bagder
Copy link
Member

bagder commented Apr 18, 2016

Thanks!

@bagder bagder closed this as completed Apr 18, 2016
bagder pushed a commit that referenced this issue Apr 21, 2016
These configure vars are modified in a curl-specific way but never
evaluated or loaded from cache, even though they are designated as
_cv_. We could either implement proper AC_CACHE_CHECKs for them, or
remove them completely.

Fixes #603 as ac_cv_func_gethostbyname is no longer clobbered, and
AC_CHECK_FUNC(gethostbyname...) will no longer spuriously succeed after
the first configure run with caching.

`ac_cv_func_strcasecmp` is curious, see #770.

`eval "ac_cv_func_$func=yes"` can still cause problems as it works in
tandem with AC_CHECK_FUNCS and then potentially modifies its result. It
would be best to rewrite this test to use a new CURL_CHECK_FUNCS macro,
which works the same as AC_CHECK_FUNCS but relies on caching the values
of curl_cv_func_* variables, without modifiying ac_cv_func_*.
@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants