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

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

Projects

None yet

2 participants

@Irfy
Irfy commented Apr 17, 2016 edited

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 Irfy pushed a commit to Irfy/curl that referenced this issue Apr 17, 2016
Irfan Adilovic configure: ac_cv_ -> curl_cv_ for write-only vars
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_*.
fdf5b5c
@bagder
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
Irfy commented Apr 18, 2016

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

@bagder bagder added a commit that referenced this issue Apr 18, 2016
@bagder bagder configure: remove check for libresolve
'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
fb823d2
@bagder
Member
bagder commented Apr 18, 2016

I just pushed this change individually to master in commit fb823d2

@bagder
Member
bagder commented Apr 18, 2016

Thanks!

@bagder bagder closed this Apr 18, 2016
@bagder bagder added a commit that referenced this issue Apr 21, 2016
@bagder Irfan Adilovic + bagder configure: ac_cv_ -> curl_cv_ for write-only vars
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_*.
d9f3b36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment