| Message ID | 20221218025934.hgg6zzq4pwvja7t4@tarta.nabijaczleweli.xyz (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [v2] redir: savefd: use F_DUPFD_CLOEXEC instead of F_DUPFD+F_SETFD, if available | expand |
наб <nabijaczleweli@nabijaczleweli.xyz> wrote: > [-- text/plain, encoding quoted-printable, charset: us-ascii, 64 lines --] > > This saves a syscall on every source file open, &c.; > F_DUPFD_CLOEXEC is a mandatory part of POSIX since Issue 7 > (Austin Group Interpretation 1003.1-2001 #171). > --- > autoconf bits adapted almost-verbatim from the st_mtim check above > > configure.ac | 13 +++++++++++++ > src/redir.c | 7 +++++++ > 2 files changed, 20 insertions(+) > > diff --git a/configure.ac b/configure.ac > index 52aa429..b870a59 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -177,6 +177,19 @@ if test "$have_st_mtim" = "yes"; then > [Define if your `struct stat' has `st_mtim']) > fi > > +dnl F_DUPFD_CLOEXEC is a mandatory part of POSIX since Issue 7 > +AC_MSG_CHECKING(for F_DUPFD_CLOEXEC) > +AC_COMPILE_IFELSE( > +[AC_LANG_PROGRAM([#include <unistd.h> > +#include <fcntl.h>], > +[return fcntl(0, F_DUPFD_CLOEXEC, 0)])], > +have_dupfd_cloexec=yes, have_dupfd_cloexec=no) > +AC_MSG_RESULT($have_dupfd_cloexec) > +if test "$have_dupfd_cloexec" = "yes"; then > + AC_DEFINE([HAVE_F_DUPFD_CLOEXEC], [1], > + [Define if your system supports F_DUPFD_CLOEXEC]) > +fi Could you please define this symbol always? That is, ... have_dupfd_cloexec=1, have_dupfd_cloexec=0) AC_DEFINE([HAVE_F_DUPFD_CLOEXEC], [${have_dupfd_cloexec}], [Set to 1 if your system supports F_DUPFD_CLOEXEC]) > err = newfd < 0 ? errno : 0; > if (err != EBADF) { > close(ofd); > if (err) > sh_error("%d: %s", from, strerror(err)); > +#if !HAVE_F_DUPFD_CLOEXEC > else > fcntl(newfd, F_SETFD, FD_CLOEXEC); > +#endif Then we could change this to else if (!HAVE_F_DUPFD_CLOEXEC) fcntl(newfd, F_SETFD, FD_CLOEXEC); Thanks,
diff --git a/configure.ac b/configure.ac index 52aa429..b870a59 100644 --- a/configure.ac +++ b/configure.ac @@ -177,6 +177,19 @@ if test "$have_st_mtim" = "yes"; then [Define if your `struct stat' has `st_mtim']) fi +dnl F_DUPFD_CLOEXEC is a mandatory part of POSIX since Issue 7 +AC_MSG_CHECKING(for F_DUPFD_CLOEXEC) +AC_COMPILE_IFELSE( +[AC_LANG_PROGRAM([#include <unistd.h> +#include <fcntl.h>], +[return fcntl(0, F_DUPFD_CLOEXEC, 0)])], +have_dupfd_cloexec=yes, have_dupfd_cloexec=no) +AC_MSG_RESULT($have_dupfd_cloexec) +if test "$have_dupfd_cloexec" = "yes"; then + AC_DEFINE([HAVE_F_DUPFD_CLOEXEC], [1], + [Define if your system supports F_DUPFD_CLOEXEC]) +fi + AC_ARG_WITH(libedit, AS_HELP_STRING(--with-libedit, [Compile with libedit support])) use_libedit= if test "$with_libedit" = "yes"; then diff --git a/src/redir.c b/src/redir.c index 5a5835c..c923bfa 100644 --- a/src/redir.c +++ b/src/redir.c @@ -446,14 +446,21 @@ savefd(int from, int ofd) int newfd; int err; +#if HAVE_F_DUPFD_CLOEXEC + newfd = fcntl(from, F_DUPFD_CLOEXEC, 10); +#else newfd = fcntl(from, F_DUPFD, 10); +#endif + err = newfd < 0 ? errno : 0; if (err != EBADF) { close(ofd); if (err) sh_error("%d: %s", from, strerror(err)); +#if !HAVE_F_DUPFD_CLOEXEC else fcntl(newfd, F_SETFD, FD_CLOEXEC); +#endif } return newfd;