threads / patch / 11723

patchsome systems don't have (and need) sys/select.h

Subject: [PATCH] some systems don't have (and need) sys/select.h

## tl;dr

9 messages between Jan 24, 2008 and Jan 26, 2008. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Robert Schiele· Jan 24, 2008, 18:34 UTC · lore

The select stuff is already in sys/time.h on on some systems like HP-UX thus we should not include sys/select.h in that case.

Signed-off-by: Robert Schiele <rschiele@gmail.com>
---
This patch replaces my previously sent patch
"HP-UX traditionally has no sys/select.h".
 Makefile          |    5 +++++
 git-compat-util.h |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)
Show changes to 2 files +7 −0

Makefile, git-compat-util.h

diff --git a/Makefile b/Makefile
index 5aac0c0..c9e54b1 100644
--- a/Makefile
+++ b/Makefile
@@ -42,6 +42,8 @@ all::
 #
 # Define NO_MKDTEMP if you don't have mkdtemp in the C library.
 #
+# Define NO_SYS_SELECT_H if you don't have sys/select.h.
+#
 # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.
 # Enable it on Windows.  By default, symrefs are still used.
 #
@@ -635,6 +637,9 @@ ifdef NO_UNSETENV
 	COMPAT_CFLAGS += -DNO_UNSETENV
 	COMPAT_OBJS += compat/unsetenv.o
 endif
+ifdef NO_SYS_SELECT_H
+	BASIC_CFLAGS += -DNO_SYS_SELECT_H
+endif
 ifdef NO_MMAP
 	COMPAT_CFLAGS += -DNO_MMAP
 	COMPAT_OBJS += compat/mmap.o
diff --git a/git-compat-util.h b/git-compat-util.h
index b6ef544..4df90cb 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -68,7 +68,9 @@
 #include <sys/poll.h>
 #include <sys/socket.h>
 #include <sys/ioctl.h>
+#ifndef NO_SYS_SELECT_H
 #include <sys/select.h>
+#endif
 #include <assert.h>
 #include <regex.h>
 #include <netinet/in.h>

-- 
1.5.2.4
Johannes Schindelin· Jan 24, 2008, 21:15 UTC · re: Robert Schiele · lore

Re: [PATCH] some systems don't have (and need) sys/select.h

Hi,
On Thu, 24 Jan 2008, Robert Schiele wrote:
> The select stuff is already in sys/time.h on on some systems like HP-UX 
> thus we should not include sys/select.h in that case.

Thank you very much, Dscho

Junio C Hamano· Jan 24, 2008, 21:17 UTC · re: Robert Schiele · lore

Re: [PATCH] some systems don't have (and need) sys/select.h

Robert Schiele <rschiele@gmail.com> writes:
> The select stuff is already in sys/time.h on on some systems like HP-UX
> thus we should not include sys/select.h in that case.
>
> Signed-off-by: Robert Schiele <rschiele@gmail.com>

The patch looks good. I'd rather try to be a bit more helpful to people who want to port git to their system that is not HP-UX, though.

How about wording it like this?
Subject: pre-POSIX.1-2001 systems do not have <sys/select.h>

POSIX.1-2001 has declaration of select(2) in <sys/select.h>, but in previous version of SUS, it is declared in <sys/time.h>, which is already included in git-compat-util.h.

This introduces NO_SYS_SELECT_H macro in the Makefile to be set on older systems, to skip inclusion of <sys/select.h> that does not exist on them.

We could check _POSIX_VERSION with 200112L and do this automatically, but earlier it was reported that the approach does not work well on some vintage of HP-UX. Other systems may get _POSIX_VERSION itself wrong. At least for now, this manual configuration is safer.

Robert Schiele· Jan 24, 2008, 21:58 UTC · re: Junio C Hamano · lore

Re: [PATCH] some systems don't have (and need) sys/select.h

On Thu, Jan 24, 2008 at 01:17:14PM -0800, Junio C Hamano wrote:
Show 28 quoted lines
> Robert Schiele <rschiele@gmail.com> writes:
> 
> > The select stuff is already in sys/time.h on on some systems like HP-UX
> > thus we should not include sys/select.h in that case.
> >
> > Signed-off-by: Robert Schiele <rschiele@gmail.com>
> 
> The patch looks good.  I'd rather try to be a bit more helpful
> to people who want to port git to their system that is not
> HP-UX, though.
> 
> How about wording it like this?
> 
> Subject: pre-POSIX.1-2001 systems do not have <sys/select.h>
> 
> POSIX.1-2001 has declaration of select(2) in <sys/select.h>, but
> in previous version of SUS, it is declared in <sys/time.h>,
> which is already included in git-compat-util.h.
> 
> This introduces NO_SYS_SELECT_H macro in the Makefile to be set
> on older systems, to skip inclusion of <sys/select.h> that does
> not exist on them.
> 
> We could check _POSIX_VERSION with 200112L and do this
> automatically, but earlier it was reported that the approach
> does not work well on some vintage of HP-UX.  Other systems may
> get _POSIX_VERSION itself wrong.  At least for now, this manual
> configuration is safer.
I have no strong opinion on the wording.  Sounds reasonable to me.
Robert
-- 
Robert Schiele
Dipl.-Wirtsch.informatiker	mailto:rschiele@gmail.com

"Quidquid latine dictum sit, altum sonatur."
Jakub Narebski· Jan 25, 2008, 11:19 UTC · re: Robert Schiele · lore

[PATCH] autoconf: Add test for sys/select.h header file

Some systems like HP-UX don't have sys/select.h; the select stuff is already present in some other headef file (e.g. sys/time.h for HP-UX).

Companion to
  "some systems don't have (and need) sys/select.h"
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
It could be alternately just squashed together with
"some systems don't have (and need) sys/select.h" by Robert Schiele.

Robert, could you please check this patch on HP-UX? It does work correctly on Linux (which has sys/select.h).

 config.mak.in |    1 +
 configure.ac  |    6 ++++++
 2 files changed, 7 insertions(+), 0 deletions(-)
Show changes to 2 files +7 −0

config.mak.in, configure.ac

diff --git a/config.mak.in b/config.mak.in
index 40b14d9..ee6c33d 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -30,6 +30,7 @@ NO_CURL=@NO_CURL@
 NO_EXPAT=@NO_EXPAT@
 NEEDS_LIBICONV=@NEEDS_LIBICONV@
 NEEDS_SOCKET=@NEEDS_SOCKET@
+NO_SYS_SELECT_H=@NO_SYS_SELECT_H@
 NO_D_INO_IN_DIRENT=@NO_D_INO_IN_DIRENT@
 NO_D_TYPE_IN_DIRENT=@NO_D_TYPE_IN_DIRENT@
 NO_SOCKADDR_STORAGE=@NO_SOCKADDR_STORAGE@
diff --git a/configure.ac b/configure.ac
index af177fd..85d7ef5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -235,6 +235,12 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 ## Checks for header files.
 AC_MSG_NOTICE([CHECKS for header files])
 #
+# Define NO_SYS_SELECT_H if you don't have sys/select.h.
+AC_CHECK_HEADER([sys/select.h],
+[NO_SYS_SELECT_H=],
+[NO_SYS_SELECT_H=UnfortunatelyYes])
+AC_SUBST(NO_SYS_SELECT_H)
+#
 # Define OLD_ICONV if your library has an old iconv(), where the second
 # (input buffer pointer) parameter is declared with type (const char **).
 AC_DEFUN([OLDICONVTEST_SRC], [[

-- 
1.5.3.7
Johannes Schindelin· Jan 25, 2008, 12:30 UTC · re: Jakub Narebski · lore

Re: [PATCH] autoconf: Add test for sys/select.h header file

Hi,
On Fri, 25 Jan 2008, Jakub Narebski wrote:
Show 14 quoted lines
> diff --git a/configure.ac b/configure.ac
> index af177fd..85d7ef5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -235,6 +235,12 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
>  ## Checks for header files.
>  AC_MSG_NOTICE([CHECKS for header files])
>  #
> +# Define NO_SYS_SELECT_H if you don't have sys/select.h.
> +AC_CHECK_HEADER([sys/select.h],
> +[NO_SYS_SELECT_H=],
> +[NO_SYS_SELECT_H=UnfortunatelyYes])
> +AC_SUBST(NO_SYS_SELECT_H)
> +#

Just because I am curious: would that not define "NO_SYS_SELECT_H" in both cases? IOW would the "ifdef NO_SYS_SELECT_H" not be triggered all the time?

Thanks, Dscho

Robert Schiele· Jan 25, 2008, 12:53 UTC · re: Johannes Schindelin · lore

Re: [PATCH] autoconf: Add test for sys/select.h header file

On Fri, Jan 25, 2008 at 12:30:27PM +0000, Johannes Schindelin wrote:
Show 22 quoted lines
> Hi,
> 
> On Fri, 25 Jan 2008, Jakub Narebski wrote:
> 
> > diff --git a/configure.ac b/configure.ac
> > index af177fd..85d7ef5 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -235,6 +235,12 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
> >  ## Checks for header files.
> >  AC_MSG_NOTICE([CHECKS for header files])
> >  #
> > +# Define NO_SYS_SELECT_H if you don't have sys/select.h.
> > +AC_CHECK_HEADER([sys/select.h],
> > +[NO_SYS_SELECT_H=],
> > +[NO_SYS_SELECT_H=UnfortunatelyYes])
> > +AC_SUBST(NO_SYS_SELECT_H)
> > +#
> 
> Just because I am curious: would that not define "NO_SYS_SELECT_H" in both 
> cases?  IOW would the "ifdef NO_SYS_SELECT_H" not be triggered all the 
> time?
No, in make the empty string is equal to not being defined.
Robert
-- 
Robert Schiele
Dipl.-Wirtsch.informatiker	mailto:rschiele@gmail.com

"Quidquid latine dictum sit, altum sonatur."
Junio C Hamano· Jan 26, 2008, 06:25 UTC · re: Jakub Narebski · lore

Re: [PATCH] autoconf: Add test for sys/select.h header file

Jakub Narebski <jnareb@gmail.com> writes:
Show 14 quoted lines
> Some systems like HP-UX don't have sys/select.h; the select stuff
> is already present in some other headef file (e.g. sys/time.h for
> HP-UX).
>
> Companion to
>   "some systems don't have (and need) sys/select.h"
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> It could be alternately just squashed together with
> "some systems don't have (and need) sys/select.h" by Robert Schiele.
>
> Robert, could you please check this patch on HP-UX? It does work
> correctly on Linux (which has sys/select.h).

The issue is not really about "do we have <sys/select.h>?" but is about "do we need to include <sys/select.h> in order to obtain declaration of select(2)?" so this is covering only a half of the issue. But I guess we will find out if the system does not offer select(2) whether <sys/select.h> is included or not, so this is not too bad.

Will apply. I do not think it would have problems but even if it does it is small and easy to revert before the final.

Thanks.
Robert Schiele· Jan 26, 2008, 09:42 UTC · re: Junio C Hamano · lore

Re: [PATCH] autoconf: Add test for sys/select.h header file

On Fri, Jan 25, 2008 at 10:25:33PM -0800, Junio C Hamano wrote:
> The issue is not really about "do we have <sys/select.h>?" but
> is about "do we need to include <sys/select.h> in order to
> obtain declaration of select(2)?" so this is covering only a

This is true. Actually HP-UX 11.23 for instance has sys/select.h but this is not intended to be included to get the select call but this is intended to be included by kernel drivers that implement the interface the kernel needs to _provide_ the select call. In that case the test determines that sys/select.h exists but it does not notice that it is not the header to include. It does not hurt though since including sys/select.h outside of a kernel driver is a no-op and we include sys/time.h which is really needed anyway.

Show 6 quoted lines
> half of the issue.  But I guess we will find out if the system
> does not offer select(2) whether <sys/select.h> is included or
> not, so this is not too bad.
> 
> Will apply.  I do not think it would have problems but even if
> it does it is small and easy to revert before the final.

At least on the systems I know it does not hurt even when it does not always exactly what it is intended to do.

Robert
-- 
Robert Schiele
Dipl.-Wirtsch.informatiker	mailto:rschiele@gmail.com

"Quidquid latine dictum sit, altum sonatur."

← back to recent threads