diff mbox series

Fix inet_ntop and inet_pton on Solaris

Message ID CAH8yC8kaWXbN+RYMJnM9em7KKW54+N07JtyS1MZk0qppD=m2BA@mail.gmail.com (mailing list archive)
State New, archived
Headers show
Series Fix inet_ntop and inet_pton on Solaris | expand

Commit Message

Jeffrey Walton Feb. 3, 2020, 3:22 p.m. UTC 
Hi Everyone,

inet_ntop and inet_pton were not being detected properly on modern on
Solaris. This patch revisits the the socket gear configuration on
SunOS and brings it up to date for Solaris 11.

According to configure.ac, the three or four functions of interest
include hstrerror, inet_ntop and inet_pton. The libraries of interest
are -lresolv -lsocket -lnsl. The configure tests now look for
inet_ntop and inet_pton in -lsocket -lnsl per the man page. If not
found, the configure tests fall back to existing behavior by searching
in -lresolv. And if not found in -lresolv, then NO_INET_NTOP and
NO_INET_PTON are set.

Here's the configure fly-by:

checking for socket... no
checking for library containing socket... no
checking for inet_ntop... no
checking for library containing inet_ntop... -lnsl
checking for inet_pton... yes
checking for hstrerror... no
checking for library containing hstrerror... -lresolv

And config.status:

$ /usr/gnu/bin/grep -E 'RESOLV|SOCKET|NSL' config.status
NEEDS_RESOLV=YesPlease
NEEDS_SOCKET=YesPlease
NEEDS_NSL=YesPlease

Jeff

Comments

Jeffrey Walton Feb. 4, 2020, midnight UTC | #1 
On Mon, Feb 3, 2020 at 10:22 AM Jeffrey Walton <noloader@gmail.com> wrote:
>
> Hi Everyone,
>
> inet_ntop and inet_pton were not being detected properly on modern on
> Solaris. This patch revisits the the socket gear configuration on
> SunOS and brings it up to date for Solaris 11.
>
> According to configure.ac, the three or four functions of interest
> include hstrerror, inet_ntop and inet_pton. The libraries of interest
> are -lresolv -lsocket -lnsl. The configure tests now look for
> inet_ntop and inet_pton in -lsocket -lnsl per the man page. If not
> found, the configure tests fall back to existing behavior by searching
> in -lresolv. And if not found in -lresolv, then NO_INET_NTOP and
> NO_INET_PTON are set.
>
> Here's the configure fly-by:
>
> checking for socket... no
> checking for library containing socket... no
> checking for inet_ntop... no
> checking for library containing inet_ntop... -lnsl
> checking for inet_pton... yes
> checking for hstrerror... no
> checking for library containing hstrerror... -lresolv
>
> And config.status:
>
> $ /usr/gnu/bin/grep -E 'RESOLV|SOCKET|NSL' config.status
> NEEDS_RESOLV=YesPlease
> NEEDS_SOCKET=YesPlease
> NEEDS_NSL=YesPlease

The patch tested good on Linux (Ubuntu 18.04), Solaris 11 (SunOS 5.11)
and Darwin (OS X 10.9).

It is the same patch used for Git Master maint branch and Git 2.25.0
release tarball.

Jeff
Jeff King Feb. 14, 2020, 6:50 a.m. UTC | #2 
On Mon, Feb 03, 2020 at 10:22:46AM -0500, Jeffrey Walton wrote:

> inet_ntop and inet_pton were not being detected properly on modern on
> Solaris. This patch revisits the the socket gear configuration on
> SunOS and brings it up to date for Solaris 11.
> 
> According to configure.ac, the three or four functions of interest
> include hstrerror, inet_ntop and inet_pton. The libraries of interest
> are -lresolv -lsocket -lnsl. The configure tests now look for
> inet_ntop and inet_pton in -lsocket -lnsl per the man page. If not
> found, the configure tests fall back to existing behavior by searching
> in -lresolv. And if not found in -lresolv, then NO_INET_NTOP and
> NO_INET_PTON are set.

This makes sense, and the patch looks plausibly correct (which is the
best I can say given my general lack of autoconf knowledge).

I was a little surprised by this hunk:

> diff --git a/Makefile b/Makefile
> index 09f98b777c..7166b19ab4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1461,15 +1461,15 @@ ifndef LIBC_CONTAINS_LIBINTL
>  	EXTLIBS += -lintl
>  endif
>  endif
> +ifdef NEEDS_RESOLV
> +	EXTLIBS += -lresolv
> +endif
>  ifdef NEEDS_SOCKET
>  	EXTLIBS += -lsocket
>  endif
>  ifdef NEEDS_NSL
>  	EXTLIBS += -lnsl
>  endif
> -ifdef NEEDS_RESOLV
> -	EXTLIBS += -lresolv
> -endif

But I guess it is trying to mimic the "-lresolv -lsocket -lnsl" order
you mentioned in the Solaris manpage. You should be able to test if
that's necessary by compiling with:

  make NEEDS_RESOLV=Yes NEEDS_SOCKET=Yes NEEDS_NSL=Yes

If so, that ordering switch could probably happen as a separate commit
before the configure.ac change.

> diff --git a/configure.ac b/configure.ac
> index 66aedb9288..b83a0e970d 100644
> --- a/configure.ac
> +++ b/configure.ac

The rest of it looks OK to me. Do you want to send it as a regular Git
patch with a commit message and your signoff (see SubmittingPatches)?

-Peff
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 09f98b777c..7166b19ab4 100644
--- a/Makefile
+++ b/Makefile
@@ -1461,15 +1461,15 @@  ifndef LIBC_CONTAINS_LIBINTL
 	EXTLIBS += -lintl
 endif
 endif
+ifdef NEEDS_RESOLV
+	EXTLIBS += -lresolv
+endif
 ifdef NEEDS_SOCKET
 	EXTLIBS += -lsocket
 endif
 ifdef NEEDS_NSL
 	EXTLIBS += -lnsl
 endif
-ifdef NEEDS_RESOLV
-	EXTLIBS += -lresolv
-endif
 ifdef NO_D_TYPE_IN_DIRENT
 	BASIC_CFLAGS += -DNO_D_TYPE_IN_DIRENT
 endif
diff --git a/configure.ac b/configure.ac
index 66aedb9288..b83a0e970d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -711,40 +711,58 @@  GIT_UNSTASH_FLAGS($ZLIB_PATH)
 
 GIT_CONF_SUBST([NO_DEFLATE_BOUND])
 
+#
+# The next few tests will define NEEDS_RESOLV, NEEDS_SOCKET or
+# NEEDS_NSL if linking with libresolv, libsocket and libnsl
+# provides some of the functions we would normally get from libc.
+NEEDS_RESOLV=
+NEEDS_SOCKET=
+NEEDS_NSL=
+
 #
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
-AC_CHECK_LIB([c], [socket],
-[NEEDS_SOCKET=],
-[NEEDS_SOCKET=YesPlease])
-GIT_CONF_SUBST([NEEDS_SOCKET])
-test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
+AC_CHECK_FUNC([socket],
+    [],
+    [AC_SEARCH_LIBS([socket], [c],
+       [NEEDS_SOCKET=],
+       [NEEDS_SOCKET=YesPlease])
+])
 
 #
-# The next few tests will define NEEDS_RESOLV if linking with
-# libresolv provides some of the functions we would normally get
-# from libc.
-NEEDS_RESOLV=
-#
-# Define NO_INET_NTOP if linking with -lresolv is not enough.
-# Solaris 2.7 in particular hos inet_ntop in -lresolv.
+# Define NO_INET_NTOP if linking with -lresolv, -lsocket and -lnsl
+# is not enough. Solaris 11 provides inet_ntop in -lsocket -lnsl.
+# Solaris 2.7 provides inet_ntop in -lresolv.
 NO_INET_NTOP=
 AC_CHECK_FUNC([inet_ntop],
     [],
-    [AC_CHECK_LIB([resolv], [inet_ntop],
-	[NEEDS_RESOLV=YesPlease],
-	[NO_INET_NTOP=YesPlease])
+    [AC_SEARCH_LIBS([inet_ntop], [socket nsl],
+       [NEEDS_SOCKET=YesPlease; NEEDS_NSL=YesPlease],
+       [AC_CHECK_FUNC([inet_ntop],
+          [],
+          [AC_SEARCH_LIBS([inet_ntop], [resolv],
+          [NEEDS_RESOLV=YesPlease],
+          [NO_INET_PTON=YesPlease])
+       ])
+    ])
 ])
 GIT_CONF_SUBST([NO_INET_NTOP])
 #
-# Define NO_INET_PTON if linking with -lresolv is not enough.
-# Solaris 2.7 in particular hos inet_pton in -lresolv.
+# Define NO_INET_PTON if linking with -lresolv, -lsocket and -lnsl
+# is not enough. Solaris 11 provides inet_pton in -lsocket -lnsl.
+# Solaris 2.7 provides inet_pton in -lresolv.
 NO_INET_PTON=
 AC_CHECK_FUNC([inet_pton],
     [],
-    [AC_CHECK_LIB([resolv], [inet_pton],
-	[NEEDS_RESOLV=YesPlease],
-	[NO_INET_PTON=YesPlease])
+    [AC_SEARCH_LIBS([inet_pton], [socket nsl],
+       [NEEDS_SOCKET=YesPlease; NEEDS_NSL=YesPlease],
+       [AC_CHECK_FUNC([inet_pton],
+          [],
+          [AC_SEARCH_LIBS([inet_pton], [resolv],
+          [NEEDS_RESOLV=YesPlease],
+          [NO_INET_PTON=YesPlease])
+       ])
+    ])
 ])
 GIT_CONF_SUBST([NO_INET_PTON])
 #
@@ -753,19 +771,26 @@  GIT_CONF_SUBST([NO_INET_PTON])
 NO_HSTRERROR=
 AC_CHECK_FUNC([hstrerror],
     [],
-    [AC_CHECK_LIB([resolv], [hstrerror],
-	[NEEDS_RESOLV=YesPlease],
-	[NO_HSTRERROR=YesPlease])
+    [AC_SEARCH_LIBS([hstrerror], [resolv],
+       [NEEDS_RESOLV=YesPlease],
+       [NO_HSTRERROR=YesPlease])
 ])
 GIT_CONF_SUBST([NO_HSTRERROR])
 
 dnl This must go after all the possible places for its initialization,
 dnl in the AC_CHECK_FUNC invocations above.
 GIT_CONF_SUBST([NEEDS_RESOLV])
+GIT_CONF_SUBST([NEEDS_SOCKET])
+GIT_CONF_SUBST([NEEDS_NSL])
+
 #
-# If any of the above tests determined that -lresolv is needed at
-# build-time, also set it here for remaining configure-time checks.
+# If any of the above tests determined that -lresolv, -lsocket or -lnsl
+# are needed at build-time, also set it here for remaining configure-time
+# checks. The Sun man pages list library order as -lresolv -lsocket -lnsl.
 test -n "$NEEDS_RESOLV" && LIBS="$LIBS -lresolv"
+test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
+test -n "$NEEDS_NSL" && LIBS="$LIBS -lnsl"
+
 
 AC_CHECK_LIB([c], [basename],
 [NEEDS_LIBGEN=],