public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Cygwin: introduce close_range
@ 2024-01-14 16:07 Christian Franke
  2024-01-14 17:30 ` Jon Turney
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Franke @ 2024-01-14 16:07 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

Recently I learned about the existence and usefulness of close_range():
https://github.com/smartmontools/smartmontools/issues/235

https://man.freebsd.org/cgi/man.cgi?query=close_range&sektion=2
https://man7.org/linux/man-pages/man2/close_range.2.html

Note that the above Linux man page is not fully correct. The include 
file "linux/close_range.h" exists, but provides only the defines. It is 
sufficient to include "unistd.h" as on FreeBSD.

The attached patch adds this to Cygwin. It does not implement the 
Linux-specific CLOSE_RANGE_UNSHARE as I have no idea how to do this :-)

-- 
Regards,
Christian


[-- Attachment #2: 0001-Cygwin-introduce-close_range.patch --]
[-- Type: text/plain, Size: 4858 bytes --]

From 2393e82a62e19e29e61ef3253e227c19ae7222eb Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Sun, 14 Jan 2024 16:54:17 +0100
Subject: [PATCH] Cygwin: introduce close_range

This function closes or sets the close-on-exec flag for a specified
range of file descriptors.  It is available on FreeBSD and Linux.

Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
 newlib/libc/include/sys/unistd.h       |  6 ++++
 winsup/cygwin/cygwin.din               |  1 +
 winsup/cygwin/include/cygwin/version.h |  3 +-
 winsup/cygwin/release/3.5.0            |  2 ++
 winsup/cygwin/syscalls.cc              | 42 ++++++++++++++++++++++++++
 winsup/doc/new-features.xml            |  4 +++
 6 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index 25532251c..f5f1963c8 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -26,6 +26,12 @@ int     chown (const char *__path, uid_t __owner, gid_t __group);
 int     chroot (const char *__path);
 #endif
 int     close (int __fildes);
+#if defined(__CYGWIN__) && (__BSD_VISIBLE || __GNU_VISIBLE)
+/* Available on FreeBSD (__BSD_VISIBLE) and Linux (__GNU_VISIBLE). */
+int     close_range (unsigned int __firstfd, unsigned int __lastfd, int __flags);
+/*      CLOSE_RANGE_UNSHARE 0x01 */ /* Linux only, not yet supported. */
+#define CLOSE_RANGE_CLOEXEC 0x02
+#endif
 #if __POSIX_VISIBLE >= 199209
 size_t	confstr (int __name, char *__buf, size_t __len);
 #endif
diff --git a/winsup/cygwin/cygwin.din b/winsup/cygwin/cygwin.din
index 9b76ce67a..9e354acc6 100644
--- a/winsup/cygwin/cygwin.din
+++ b/winsup/cygwin/cygwin.din
@@ -347,6 +347,7 @@ clog10l NOSIGFE
 clogf NOSIGFE
 clogl NOSIGFE
 close SIGFE
+close_range SIGFE
 closedir SIGFE
 closelog SIGFE
 cnd_broadcast SIGFE
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index c8177c2b1..3036878c4 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -484,12 +484,13 @@ details. */
   347: Add c16rtomb, c32rtomb, mbrtoc16, mbrtoc32.
   348: Add c8rtomb, mbrtoc.
   349: Add fallocate.
+  350: Add close_range.
 
   Note that we forgot to bump the api for ualarm, strtoll, strtoull,
   sigaltstack, sethostname. */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 349
+#define CYGWIN_VERSION_API_MINOR 350
 
 /* There is also a compatibity version number associated with the shared memory
    regions.  It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/release/3.5.0 b/winsup/cygwin/release/3.5.0
index d0a6c2fc8..6209064a6 100644
--- a/winsup/cygwin/release/3.5.0
+++ b/winsup/cygwin/release/3.5.0
@@ -43,6 +43,8 @@ What's new:
 
 - New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 
+- New API call: close_range (available on FreeBSD and Linux).
+
 - New API call: fallocate (Linux-specific).
 
 - Implement OSS-based sound mixer device (/dev/mixer).
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 486db1db6..aba864475 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -85,6 +85,48 @@ close_all_files (bool norelease)
   cygheap->fdtab.unlock ();
 }
 
+/* Close or set the close-on-exec flag for all open file descriptors
+   from firstfd to lastfd.
+   Available on FreeBSD since 13 and Linux since 5.9 */
+extern "C" int
+close_range (unsigned int firstfd, unsigned int lastfd, int flags)
+{
+  pthread_testcancel ();
+
+  if (!(firstfd <= lastfd && (!flags || flags == CLOSE_RANGE_CLOEXEC)))
+    {
+      set_errno (EINVAL);
+      return -1;
+    }
+
+  cygheap->fdtab.lock ();
+
+  int size = (int) (lastfd < cygheap->fdtab.size ? lastfd + 1 :
+		    cygheap->fdtab.size);
+
+  for (int i = (int) firstfd; i < size; i++)
+    {
+      cygheap_fdget cfd (i, false, false);
+      if (cfd < 0)
+	continue;
+
+      if (!flags)
+	{
+	  syscall_printf ("closing fd %d", i);
+	  cfd->close_with_arch ();
+	  cfd.release ();
+	}
+      else /* CLOSE_RANGE_CLOEXEC */
+	{
+	  syscall_printf ("set FD_CLOEXEC on fd %d", i);
+	  cfd->fcntl (F_SETFD, FD_CLOEXEC);
+	}
+    }
+
+  cygheap->fdtab.unlock ();
+  return 0;
+}
+
 extern "C" int
 dup (int fd)
 {
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 6ae420031..0abe1c41c 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -74,6 +74,10 @@ posix_spawn_file_actions_addfchdir_np.
 New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 </para></listitem>
 
+<listitem><para>
+New API call: close_range (available on FreeBSD and Linux).
+</para></listitem>
+
 <listitem><para>
 New API call: fallocate (Linux-specific).
 </para></listitem>
-- 
2.42.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-14 16:07 [PATCH] Cygwin: introduce close_range Christian Franke
@ 2024-01-14 17:30 ` Jon Turney
  2024-01-14 18:53   ` Christian Franke
  0 siblings, 1 reply; 11+ messages in thread
From: Jon Turney @ 2024-01-14 17:30 UTC (permalink / raw)
  To: Christian Franke; +Cc: cygwin-patches

On 14/01/2024 16:07, Christian Franke wrote:
> Recently I learned about the existence and usefulness of close_range():
> https://github.com/smartmontools/smartmontools/issues/235
> 
> https://man.freebsd.org/cgi/man.cgi?query=close_range&sektion=2
> https://man7.org/linux/man-pages/man2/close_range.2.html
> 
> Note that the above Linux man page is not fully correct. The include 
> file "linux/close_range.h" exists, but provides only the defines. It is 
> sufficient to include "unistd.h" as on FreeBSD.
> 
> The attached patch adds this to Cygwin. It does not implement the 
> Linux-specific CLOSE_RANGE_UNSHARE as I have no idea how to do this :-)

This API should also be mentioned in the
"System interfaces compatible with GNU or Linux extensions" section of 
doc/posix.xml


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-14 17:30 ` Jon Turney
@ 2024-01-14 18:53   ` Christian Franke
  2024-01-15  8:56     ` Christian Franke
  2024-01-15  9:50     ` Corinna Vinschen
  0 siblings, 2 replies; 11+ messages in thread
From: Christian Franke @ 2024-01-14 18:53 UTC (permalink / raw)
  To: cygwin-patches

Jon Turney wrote:
> On 14/01/2024 16:07, Christian Franke wrote:
>> Recently I learned about the existence and usefulness of close_range():
>> https://github.com/smartmontools/smartmontools/issues/235
>>
>> https://man.freebsd.org/cgi/man.cgi?query=close_range&sektion=2
>> https://man7.org/linux/man-pages/man2/close_range.2.html
>>
>> Note that the above Linux man page is not fully correct. The include 
>> file "linux/close_range.h" exists, but provides only the defines. It 
>> is sufficient to include "unistd.h" as on FreeBSD.
>>
>> The attached patch adds this to Cygwin. It does not implement the 
>> Linux-specific CLOSE_RANGE_UNSHARE as I have no idea how to do this :-)
>
> This API should also be mentioned in the
> "System interfaces compatible with GNU or Linux extensions" section of 
> doc/posix.xml
>
>

Thanks for the info. I used the recent "Cygwin: introduce fallocate(2)" 
patch as a blueprint for which other files should be changed (fallocate 
is also missing in the posix.xml file).

I will provide a new patch soon which also fixes an unlikely but 
possible corner case: Pass a value larger than MAX_INT as lower limit.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-14 18:53   ` Christian Franke
@ 2024-01-15  8:56     ` Christian Franke
  2024-01-15 10:44       ` Corinna Vinschen
  2024-01-15  9:50     ` Corinna Vinschen
  1 sibling, 1 reply; 11+ messages in thread
From: Christian Franke @ 2024-01-15  8:56 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Christian Franke wrote:
> Jon Turney wrote:
>> On 14/01/2024 16:07, Christian Franke wrote:
>>> Recently I learned about the existence and usefulness of close_range():
>>> https://github.com/smartmontools/smartmontools/issues/235
>>>
>>> https://man.freebsd.org/cgi/man.cgi?query=close_range&sektion=2
>>> https://man7.org/linux/man-pages/man2/close_range.2.html
>>>
>>> Note that the above Linux man page is not fully correct. The include 
>>> file "linux/close_range.h" exists, but provides only the defines. It 
>>> is sufficient to include "unistd.h" as on FreeBSD.
>>>
>>> The attached patch adds this to Cygwin. It does not implement the 
>>> Linux-specific CLOSE_RANGE_UNSHARE as I have no idea how to do this :-)
>>
>> This API should also be mentioned in the
>> "System interfaces compatible with GNU or Linux extensions" section 
>> of doc/posix.xml
>>
>>
>
> Thanks for the info. I used the recent "Cygwin: introduce 
> fallocate(2)" patch as a blueprint for which other files should be 
> changed (fallocate is also missing in the posix.xml file).
>
> I will provide a new patch soon which also fixes an unlikely but 
> possible corner case: Pass a value larger than MAX_INT as lower limit.
>

Attached. I also decided to simply ignore CLOSE_RANGE_UNSHARE for now.


[-- Attachment #2: 0001-Cygwin-introduce-close_range-2.patch --]
[-- Type: text/plain, Size: 6220 bytes --]

From f7704bf77a926e53e8200528ab6abc1c9fdca511 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Mon, 15 Jan 2024 09:52:50 +0100
Subject: [PATCH] Cygwin: introduce close_range(2)

This function closes or sets the close-on-exec flag for a specified
range of file descriptors.  It is available on FreeBSD and Linux.

Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
 newlib/libc/include/sys/unistd.h       |  9 ++++++
 winsup/cygwin/cygwin.din               |  1 +
 winsup/cygwin/include/cygwin/version.h |  3 +-
 winsup/cygwin/release/3.5.0            |  2 ++
 winsup/cygwin/syscalls.cc              | 43 ++++++++++++++++++++++++++
 winsup/doc/new-features.xml            |  4 +++
 winsup/doc/posix.xml                   |  5 +++
 7 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index 25532251c..0a31544ed 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -26,6 +26,15 @@ int     chown (const char *__path, uid_t __owner, gid_t __group);
 int     chroot (const char *__path);
 #endif
 int     close (int __fildes);
+#if defined(__CYGWIN__) && (__BSD_VISIBLE || __GNU_VISIBLE)
+/* Available on FreeBSD (__BSD_VISIBLE) and Linux (__GNU_VISIBLE). */
+int     close_range (unsigned int __firstfd, unsigned int __lastfd, int __flags);
+#if __GNU_VISIBLE
+/* Linux-specific, ignored by Cygwin. */
+#define CLOSE_RANGE_UNSHARE (1 << 1)
+#endif
+#define CLOSE_RANGE_CLOEXEC (1 << 2)
+#endif
 #if __POSIX_VISIBLE >= 199209
 size_t	confstr (int __name, char *__buf, size_t __len);
 #endif
diff --git a/winsup/cygwin/cygwin.din b/winsup/cygwin/cygwin.din
index 9b76ce67a..9e354acc6 100644
--- a/winsup/cygwin/cygwin.din
+++ b/winsup/cygwin/cygwin.din
@@ -347,6 +347,7 @@ clog10l NOSIGFE
 clogf NOSIGFE
 clogl NOSIGFE
 close SIGFE
+close_range SIGFE
 closedir SIGFE
 closelog SIGFE
 cnd_broadcast SIGFE
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index c8177c2b1..3036878c4 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -484,12 +484,13 @@ details. */
   347: Add c16rtomb, c32rtomb, mbrtoc16, mbrtoc32.
   348: Add c8rtomb, mbrtoc.
   349: Add fallocate.
+  350: Add close_range.
 
   Note that we forgot to bump the api for ualarm, strtoll, strtoull,
   sigaltstack, sethostname. */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 349
+#define CYGWIN_VERSION_API_MINOR 350
 
 /* There is also a compatibity version number associated with the shared memory
    regions.  It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/release/3.5.0 b/winsup/cygwin/release/3.5.0
index d0a6c2fc8..6209064a6 100644
--- a/winsup/cygwin/release/3.5.0
+++ b/winsup/cygwin/release/3.5.0
@@ -43,6 +43,8 @@ What's new:
 
 - New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 
+- New API call: close_range (available on FreeBSD and Linux).
+
 - New API call: fallocate (Linux-specific).
 
 - Implement OSS-based sound mixer device (/dev/mixer).
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 486db1db6..2e1b56b7f 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -85,6 +85,49 @@ close_all_files (bool norelease)
   cygheap->fdtab.unlock ();
 }
 
+/* Close or set the close-on-exec flag for all open file descriptors
+   from firstfd to lastfd.  CLOSE_RANGE_UNSHARE is ignored.
+   Available on FreeBSD since 13 and Linux since 5.9 */
+extern "C" int
+close_range (unsigned int firstfd, unsigned int lastfd, int flags)
+{
+  pthread_testcancel ();
+
+  if (!(firstfd <= lastfd &&
+      !(flags & ~(CLOSE_RANGE_UNSHARE | CLOSE_RANGE_CLOEXEC))))
+    {
+      set_errno (EINVAL);
+      return -1;
+    }
+
+  cygheap->fdtab.lock ();
+
+  unsigned int size = (lastfd < cygheap->fdtab.size ? lastfd + 1 :
+		      cygheap->fdtab.size);
+
+  for (unsigned int i = firstfd; i < size; i++)
+    {
+      cygheap_fdget cfd ((int) i, false, false);
+      if (cfd < 0)
+	continue;
+
+      if (flags & CLOSE_RANGE_CLOEXEC)
+	{
+	  syscall_printf ("set FD_CLOEXEC on fd %u", i);
+	  cfd->fcntl (F_SETFD, FD_CLOEXEC);
+	}
+      else
+	{
+	  syscall_printf ("closing fd %u", i);
+	  cfd->close_with_arch ();
+	  cfd.release ();
+	}
+    }
+
+  cygheap->fdtab.unlock ();
+  return 0;
+}
+
 extern "C" int
 dup (int fd)
 {
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 6ae420031..0abe1c41c 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -74,6 +74,10 @@ posix_spawn_file_actions_addfchdir_np.
 New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 </para></listitem>
 
+<listitem><para>
+New API call: close_range (available on FreeBSD and Linux).
+</para></listitem>
+
 <listitem><para>
 New API call: fallocate (Linux-specific).
 </para></listitem>
diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml
index 151aeb9fe..4ece761a8 100644
--- a/winsup/doc/posix.xml
+++ b/winsup/doc/posix.xml
@@ -1143,6 +1143,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
     cfmakeraw
     cfsetspeed
     clearerr_unlocked
+    close_range
     daemon
     dn_comp
     dn_expand
@@ -1297,6 +1298,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
     clog10
     clog10f
     clog10l
+    close_range			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
     crypt_r			(available in external "crypt" library)
     dladdr			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
     dremf
@@ -1655,6 +1657,9 @@ CLOCK_REALTIME and CLOCK_MONOTONIC.  <function>clock_setres</function>,
 <function>clock_settime</function>, and <function>timer_create</function>
 currently support only CLOCK_REALTIME.</para>
 
+<para><function>close_range</function> ignores the Linux-specific flag
+CLOSE_RANGE_UNSHARE.</para>
+
 <para>POSIX file locks via <function>fcntl</function> or
 <function>lockf</function>, as well as BSD <function>flock</function> locks
 are advisory locks.  They don't interact with Windows mandatory locks, nor
-- 
2.42.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-14 18:53   ` Christian Franke
  2024-01-15  8:56     ` Christian Franke
@ 2024-01-15  9:50     ` Corinna Vinschen
  1 sibling, 0 replies; 11+ messages in thread
From: Corinna Vinschen @ 2024-01-15  9:50 UTC (permalink / raw)
  To: cygwin-patches

On Jan 14 19:53, Christian Franke wrote:
> Jon Turney wrote:
> > On 14/01/2024 16:07, Christian Franke wrote:
> > > Recently I learned about the existence and usefulness of close_range():
> > > https://github.com/smartmontools/smartmontools/issues/235
> > > 
> > > https://man.freebsd.org/cgi/man.cgi?query=close_range&sektion=2
> > > https://man7.org/linux/man-pages/man2/close_range.2.html
> > > 
> > > Note that the above Linux man page is not fully correct. The include
> > > file "linux/close_range.h" exists, but provides only the defines. It
> > > is sufficient to include "unistd.h" as on FreeBSD.
> > > 
> > > The attached patch adds this to Cygwin. It does not implement the
> > > Linux-specific CLOSE_RANGE_UNSHARE as I have no idea how to do this
> > > :-)
> > 
> > This API should also be mentioned in the
> > "System interfaces compatible with GNU or Linux extensions" section of
> > doc/posix.xml
> > 
> > 
> 
> Thanks for the info. I used the recent "Cygwin: introduce fallocate(2)"
> patch as a blueprint for which other files should be changed (fallocate is
> also missing in the posix.xml file).

Oops, thanks for notifying. I'll add it in a bit...


Corinna

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-15  8:56     ` Christian Franke
@ 2024-01-15 10:44       ` Corinna Vinschen
  2024-01-15 11:19         ` Christian Franke
  0 siblings, 1 reply; 11+ messages in thread
From: Corinna Vinschen @ 2024-01-15 10:44 UTC (permalink / raw)
  To: cygwin-patches

Hi Christian,

On Jan 15 09:56, Christian Franke wrote:
> Christian Franke wrote:
> > Jon Turney wrote:
> > > On 14/01/2024 16:07, Christian Franke wrote:
> > > > Recently I learned about the existence and usefulness of close_range():
> > > > https://github.com/smartmontools/smartmontools/issues/235
> > > > 
> > > > https://man.freebsd.org/cgi/man.cgi?query=close_range&sektion=2
> > > > https://man7.org/linux/man-pages/man2/close_range.2.html
> > > > 
> > > > Note that the above Linux man page is not fully correct. The
> > > > include file "linux/close_range.h" exists, but provides only the
> > > > defines. It is sufficient to include "unistd.h" as on FreeBSD.
> > > > 
> > > > The attached patch adds this to Cygwin. It does not implement
> > > > the Linux-specific CLOSE_RANGE_UNSHARE as I have no idea how to
> > > > do this :-)
> > > 
> > > This API should also be mentioned in the
> > > "System interfaces compatible with GNU or Linux extensions" section
> > > of doc/posix.xml
> > > 
> > > 
> > 
> > Thanks for the info. I used the recent "Cygwin: introduce fallocate(2)"
> > patch as a blueprint for which other files should be changed (fallocate
> > is also missing in the posix.xml file).
> > 
> > I will provide a new patch soon which also fixes an unlikely but
> > possible corner case: Pass a value larger than MAX_INT as lower limit.
> > 
> 
> Attached. I also decided to simply ignore CLOSE_RANGE_UNSHARE for now.

After reading up on this issue, I think we should not ignore
CLOSE_RANGE_UNSHARE, but quite explicitely not implement it as a valid
flag.

The whole idea behind CLOSE_RANGE_UNSHARE depends on the way the Linux
kernel creates threads and (forked) processes and the fact that it has a
wide range of ways to share parts of the execution context between
parent and child process/thread.

So on Linux, a process/thread can actually decide if they share or not
share the descriptor table with the created process/thread.  That's
what the CLONE_FILES flag to clone(2) and unshare(2) manage.

However, just as in FreeBSD, there's no such thing in Cygwin.  Threads
always share descriptors, processes never share file desriptors.

The bottom line is, I think the decision of the FreeBSD developer not to
expose the CLOSE_RANGE_UNSHARE flag at all, was the right decision.

We should not claim that we even remotely have a way of doing this
the Linux way.

Does that make sense?

Apart from this little thing I think the patch is nice.


Thanks,
Corinna



> From f7704bf77a926e53e8200528ab6abc1c9fdca511 Mon Sep 17 00:00:00 2001
> From: Christian Franke <christian.franke@t-online.de>
> Date: Mon, 15 Jan 2024 09:52:50 +0100
> Subject: [PATCH] Cygwin: introduce close_range(2)
> 
> This function closes or sets the close-on-exec flag for a specified
> range of file descriptors.  It is available on FreeBSD and Linux.
> 
> Signed-off-by: Christian Franke <christian.franke@t-online.de>
> ---
>  newlib/libc/include/sys/unistd.h       |  9 ++++++
>  winsup/cygwin/cygwin.din               |  1 +
>  winsup/cygwin/include/cygwin/version.h |  3 +-
>  winsup/cygwin/release/3.5.0            |  2 ++
>  winsup/cygwin/syscalls.cc              | 43 ++++++++++++++++++++++++++
>  winsup/doc/new-features.xml            |  4 +++
>  winsup/doc/posix.xml                   |  5 +++
>  7 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
> index 25532251c..0a31544ed 100644
> --- a/newlib/libc/include/sys/unistd.h
> +++ b/newlib/libc/include/sys/unistd.h
> @@ -26,6 +26,15 @@ int     chown (const char *__path, uid_t __owner, gid_t __group);
>  int     chroot (const char *__path);
>  #endif
>  int     close (int __fildes);
> +#if defined(__CYGWIN__) && (__BSD_VISIBLE || __GNU_VISIBLE)
> +/* Available on FreeBSD (__BSD_VISIBLE) and Linux (__GNU_VISIBLE). */
> +int     close_range (unsigned int __firstfd, unsigned int __lastfd, int __flags);
> +#if __GNU_VISIBLE
> +/* Linux-specific, ignored by Cygwin. */
> +#define CLOSE_RANGE_UNSHARE (1 << 1)
> +#endif
> +#define CLOSE_RANGE_CLOEXEC (1 << 2)
> +#endif
>  #if __POSIX_VISIBLE >= 199209
>  size_t	confstr (int __name, char *__buf, size_t __len);
>  #endif
> diff --git a/winsup/cygwin/cygwin.din b/winsup/cygwin/cygwin.din
> index 9b76ce67a..9e354acc6 100644
> --- a/winsup/cygwin/cygwin.din
> +++ b/winsup/cygwin/cygwin.din
> @@ -347,6 +347,7 @@ clog10l NOSIGFE
>  clogf NOSIGFE
>  clogl NOSIGFE
>  close SIGFE
> +close_range SIGFE
>  closedir SIGFE
>  closelog SIGFE
>  cnd_broadcast SIGFE
> diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
> index c8177c2b1..3036878c4 100644
> --- a/winsup/cygwin/include/cygwin/version.h
> +++ b/winsup/cygwin/include/cygwin/version.h
> @@ -484,12 +484,13 @@ details. */
>    347: Add c16rtomb, c32rtomb, mbrtoc16, mbrtoc32.
>    348: Add c8rtomb, mbrtoc.
>    349: Add fallocate.
> +  350: Add close_range.
>  
>    Note that we forgot to bump the api for ualarm, strtoll, strtoull,
>    sigaltstack, sethostname. */
>  
>  #define CYGWIN_VERSION_API_MAJOR 0
> -#define CYGWIN_VERSION_API_MINOR 349
> +#define CYGWIN_VERSION_API_MINOR 350
>  
>  /* There is also a compatibity version number associated with the shared memory
>     regions.  It is incremented when incompatible changes are made to the shared
> diff --git a/winsup/cygwin/release/3.5.0 b/winsup/cygwin/release/3.5.0
> index d0a6c2fc8..6209064a6 100644
> --- a/winsup/cygwin/release/3.5.0
> +++ b/winsup/cygwin/release/3.5.0
> @@ -43,6 +43,8 @@ What's new:
>  
>  - New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
>  
> +- New API call: close_range (available on FreeBSD and Linux).
> +
>  - New API call: fallocate (Linux-specific).
>  
>  - Implement OSS-based sound mixer device (/dev/mixer).
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 486db1db6..2e1b56b7f 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -85,6 +85,49 @@ close_all_files (bool norelease)
>    cygheap->fdtab.unlock ();
>  }
>  
> +/* Close or set the close-on-exec flag for all open file descriptors
> +   from firstfd to lastfd.  CLOSE_RANGE_UNSHARE is ignored.
> +   Available on FreeBSD since 13 and Linux since 5.9 */
> +extern "C" int
> +close_range (unsigned int firstfd, unsigned int lastfd, int flags)
> +{
> +  pthread_testcancel ();
> +
> +  if (!(firstfd <= lastfd &&
> +      !(flags & ~(CLOSE_RANGE_UNSHARE | CLOSE_RANGE_CLOEXEC))))
> +    {
> +      set_errno (EINVAL);
> +      return -1;
> +    }
> +
> +  cygheap->fdtab.lock ();
> +
> +  unsigned int size = (lastfd < cygheap->fdtab.size ? lastfd + 1 :
> +		      cygheap->fdtab.size);
> +
> +  for (unsigned int i = firstfd; i < size; i++)
> +    {
> +      cygheap_fdget cfd ((int) i, false, false);
> +      if (cfd < 0)
> +	continue;
> +
> +      if (flags & CLOSE_RANGE_CLOEXEC)
> +	{
> +	  syscall_printf ("set FD_CLOEXEC on fd %u", i);
> +	  cfd->fcntl (F_SETFD, FD_CLOEXEC);
> +	}
> +      else
> +	{
> +	  syscall_printf ("closing fd %u", i);
> +	  cfd->close_with_arch ();
> +	  cfd.release ();
> +	}
> +    }
> +
> +  cygheap->fdtab.unlock ();
> +  return 0;
> +}
> +
>  extern "C" int
>  dup (int fd)
>  {
> diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
> index 6ae420031..0abe1c41c 100644
> --- a/winsup/doc/new-features.xml
> +++ b/winsup/doc/new-features.xml
> @@ -74,6 +74,10 @@ posix_spawn_file_actions_addfchdir_np.
>  New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
>  </para></listitem>
>  
> +<listitem><para>
> +New API call: close_range (available on FreeBSD and Linux).
> +</para></listitem>
> +
>  <listitem><para>
>  New API call: fallocate (Linux-specific).
>  </para></listitem>
> diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml
> index 151aeb9fe..4ece761a8 100644
> --- a/winsup/doc/posix.xml
> +++ b/winsup/doc/posix.xml
> @@ -1143,6 +1143,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
>      cfmakeraw
>      cfsetspeed
>      clearerr_unlocked
> +    close_range
>      daemon
>      dn_comp
>      dn_expand
> @@ -1297,6 +1298,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
>      clog10
>      clog10f
>      clog10l
> +    close_range			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
>      crypt_r			(available in external "crypt" library)
>      dladdr			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
>      dremf
> @@ -1655,6 +1657,9 @@ CLOCK_REALTIME and CLOCK_MONOTONIC.  <function>clock_setres</function>,
>  <function>clock_settime</function>, and <function>timer_create</function>
>  currently support only CLOCK_REALTIME.</para>
>  
> +<para><function>close_range</function> ignores the Linux-specific flag
> +CLOSE_RANGE_UNSHARE.</para>
> +
>  <para>POSIX file locks via <function>fcntl</function> or
>  <function>lockf</function>, as well as BSD <function>flock</function> locks
>  are advisory locks.  They don't interact with Windows mandatory locks, nor
> -- 
> 2.42.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-15 10:44       ` Corinna Vinschen
@ 2024-01-15 11:19         ` Christian Franke
  2024-01-15 12:07           ` Corinna Vinschen
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Franke @ 2024-01-15 11:19 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 2450 bytes --]

Corinna Vinschen wrote:
> Hi Christian,
>
> On Jan 15 09:56, Christian Franke wrote:
>> Christian Franke wrote:
>>> Jon Turney wrote:
>>>> On 14/01/2024 16:07, Christian Franke wrote:
>>>>> Recently I learned about the existence and usefulness of close_range():
>>>>> https://github.com/smartmontools/smartmontools/issues/235
>>>>>
>>>>> https://man.freebsd.org/cgi/man.cgi?query=close_range&sektion=2
>>>>> https://man7.org/linux/man-pages/man2/close_range.2.html
>>>>>
>>>>> Note that the above Linux man page is not fully correct. The
>>>>> include file "linux/close_range.h" exists, but provides only the
>>>>> defines. It is sufficient to include "unistd.h" as on FreeBSD.
>>>>>
>>>>> The attached patch adds this to Cygwin. It does not implement
>>>>> the Linux-specific CLOSE_RANGE_UNSHARE as I have no idea how to
>>>>> do this :-)
>>>> This API should also be mentioned in the
>>>> "System interfaces compatible with GNU or Linux extensions" section
>>>> of doc/posix.xml
>>>>
>>>>
>>> Thanks for the info. I used the recent "Cygwin: introduce fallocate(2)"
>>> patch as a blueprint for which other files should be changed (fallocate
>>> is also missing in the posix.xml file).
>>>
>>> I will provide a new patch soon which also fixes an unlikely but
>>> possible corner case: Pass a value larger than MAX_INT as lower limit.
>>>
>> Attached. I also decided to simply ignore CLOSE_RANGE_UNSHARE for now.
> After reading up on this issue, I think we should not ignore
> CLOSE_RANGE_UNSHARE, but quite explicitely not implement it as a valid
> flag.
>
> The whole idea behind CLOSE_RANGE_UNSHARE depends on the way the Linux
> kernel creates threads and (forked) processes and the fact that it has a
> wide range of ways to share parts of the execution context between
> parent and child process/thread.
>
> So on Linux, a process/thread can actually decide if they share or not
> share the descriptor table with the created process/thread.  That's
> what the CLONE_FILES flag to clone(2) and unshare(2) manage.
>
> However, just as in FreeBSD, there's no such thing in Cygwin.  Threads
> always share descriptors, processes never share file desriptors.
>
> The bottom line is, I think the decision of the FreeBSD developer not to
> expose the CLOSE_RANGE_UNSHARE flag at all, was the right decision.
>
> We should not claim that we even remotely have a way of doing this
> the Linux way.
>
> Does that make sense?

Yes - new patch attached.


[-- Attachment #2: 0001-Cygwin-introduce-close_range-2.patch --]
[-- Type: text/plain, Size: 6173 bytes --]

From 3d538a35732b1e8cd3d7e7921e06dfdffc6d28db Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Mon, 15 Jan 2024 12:13:30 +0100
Subject: [PATCH] Cygwin: introduce close_range(2)

This function closes or sets the close-on-exec flag for a specified
range of file descriptors.  It is available on FreeBSD and Linux.

Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
 newlib/libc/include/sys/unistd.h       |  6 ++++
 winsup/cygwin/cygwin.din               |  1 +
 winsup/cygwin/include/cygwin/version.h |  3 +-
 winsup/cygwin/release/3.5.0            |  2 ++
 winsup/cygwin/syscalls.cc              | 42 ++++++++++++++++++++++++++
 winsup/doc/new-features.xml            |  4 +++
 winsup/doc/posix.xml                   |  5 +++
 7 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index 25532251c..00901540f 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -26,6 +26,12 @@ int     chown (const char *__path, uid_t __owner, gid_t __group);
 int     chroot (const char *__path);
 #endif
 int     close (int __fildes);
+#if defined(__CYGWIN__) && (__BSD_VISIBLE || __GNU_VISIBLE)
+/* Available on FreeBSD (__BSD_VISIBLE) and Linux (__GNU_VISIBLE). */
+int     close_range (unsigned int __firstfd, unsigned int __lastfd, int __flags);
+/*      CLOSE_RANGE_UNSHARE (1 << 1) */ /* Linux-specific, not supported. */
+#define CLOSE_RANGE_CLOEXEC (1 << 2)
+#endif
 #if __POSIX_VISIBLE >= 199209
 size_t	confstr (int __name, char *__buf, size_t __len);
 #endif
diff --git a/winsup/cygwin/cygwin.din b/winsup/cygwin/cygwin.din
index 9b76ce67a..9e354acc6 100644
--- a/winsup/cygwin/cygwin.din
+++ b/winsup/cygwin/cygwin.din
@@ -347,6 +347,7 @@ clog10l NOSIGFE
 clogf NOSIGFE
 clogl NOSIGFE
 close SIGFE
+close_range SIGFE
 closedir SIGFE
 closelog SIGFE
 cnd_broadcast SIGFE
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index c8177c2b1..3036878c4 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -484,12 +484,13 @@ details. */
   347: Add c16rtomb, c32rtomb, mbrtoc16, mbrtoc32.
   348: Add c8rtomb, mbrtoc.
   349: Add fallocate.
+  350: Add close_range.
 
   Note that we forgot to bump the api for ualarm, strtoll, strtoull,
   sigaltstack, sethostname. */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 349
+#define CYGWIN_VERSION_API_MINOR 350
 
 /* There is also a compatibity version number associated with the shared memory
    regions.  It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/release/3.5.0 b/winsup/cygwin/release/3.5.0
index d0a6c2fc8..6209064a6 100644
--- a/winsup/cygwin/release/3.5.0
+++ b/winsup/cygwin/release/3.5.0
@@ -43,6 +43,8 @@ What's new:
 
 - New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 
+- New API call: close_range (available on FreeBSD and Linux).
+
 - New API call: fallocate (Linux-specific).
 
 - Implement OSS-based sound mixer device (/dev/mixer).
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 486db1db6..9d88b60b0 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -85,6 +85,48 @@ close_all_files (bool norelease)
   cygheap->fdtab.unlock ();
 }
 
+/* Close or set the close-on-exec flag for all open file descriptors
+   from firstfd to lastfd.  CLOSE_RANGE_UNSHARE is not supported.
+   Available on FreeBSD since 13 and Linux since 5.9 */
+extern "C" int
+close_range (unsigned int firstfd, unsigned int lastfd, int flags)
+{
+  pthread_testcancel ();
+
+  if (!(firstfd <= lastfd && !(flags & ~CLOSE_RANGE_CLOEXEC)))
+    {
+      set_errno (EINVAL);
+      return -1;
+    }
+
+  cygheap->fdtab.lock ();
+
+  unsigned int size = (lastfd < cygheap->fdtab.size ? lastfd + 1 :
+		      cygheap->fdtab.size);
+
+  for (unsigned int i = firstfd; i < size; i++)
+    {
+      cygheap_fdget cfd ((int) i, false, false);
+      if (cfd < 0)
+	continue;
+
+      if (flags & CLOSE_RANGE_CLOEXEC)
+	{
+	  syscall_printf ("set FD_CLOEXEC on fd %u", i);
+	  cfd->fcntl (F_SETFD, FD_CLOEXEC);
+	}
+      else
+	{
+	  syscall_printf ("closing fd %u", i);
+	  cfd->close_with_arch ();
+	  cfd.release ();
+	}
+    }
+
+  cygheap->fdtab.unlock ();
+  return 0;
+}
+
 extern "C" int
 dup (int fd)
 {
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 6ae420031..0abe1c41c 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -74,6 +74,10 @@ posix_spawn_file_actions_addfchdir_np.
 New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 </para></listitem>
 
+<listitem><para>
+New API call: close_range (available on FreeBSD and Linux).
+</para></listitem>
+
 <listitem><para>
 New API call: fallocate (Linux-specific).
 </para></listitem>
diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml
index 1a4eee1ab..89056915b 100644
--- a/winsup/doc/posix.xml
+++ b/winsup/doc/posix.xml
@@ -1143,6 +1143,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
     cfmakeraw
     cfsetspeed
     clearerr_unlocked
+    close_range
     daemon
     dn_comp
     dn_expand
@@ -1297,6 +1298,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
     clog10
     clog10f
     clog10l
+    close_range			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
     crypt_r			(available in external "crypt" library)
     dladdr			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
     dremf
@@ -1656,6 +1658,9 @@ CLOCK_REALTIME and CLOCK_MONOTONIC.  <function>clock_setres</function>,
 <function>clock_settime</function>, and <function>timer_create</function>
 currently support only CLOCK_REALTIME.</para>
 
+<para><function>close_range</function> does not support the Linux-specific
+flag CLOSE_RANGE_UNSHARE.</para>
+
 <para>POSIX file locks via <function>fcntl</function> or
 <function>lockf</function>, as well as BSD <function>flock</function> locks
 are advisory locks.  They don't interact with Windows mandatory locks, nor
-- 
2.42.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-15 11:19         ` Christian Franke
@ 2024-01-15 12:07           ` Corinna Vinschen
  2024-01-15 12:08             ` Corinna Vinschen
  0 siblings, 1 reply; 11+ messages in thread
From: Corinna Vinschen @ 2024-01-15 12:07 UTC (permalink / raw)
  To: cygwin-patches

Sorry Christian, but..

I was just going to push this patch when I realized that we now have
two lines of debug output per affected file descriptor:

On Jan 15 12:19, Christian Franke wrote:
> +  for (unsigned int i = firstfd; i < size; i++)
> +    {
> +      cygheap_fdget cfd ((int) i, false, false);
> +      if (cfd < 0)
> +	continue;
> +
> +      if (flags & CLOSE_RANGE_CLOEXEC)
> +	{
> +	  syscall_printf ("set FD_CLOEXEC on fd %u", i);
> +	  cfd->fcntl (F_SETFD, FD_CLOEXEC);

fhandler::set_close_on_exec() already prints this:

  debug_printf ("set close_on_exec for %s to %d", get_name (), val);

> +	}
> +      else
> +	{
> +	  syscall_printf ("closing fd %u", i);
> +	  cfd->close_with_arch ();

fhandler::close() already prints this:

  syscall_printf ("closing '%s' handle %p", get_name (), get_handle ());

Shan't we drop the syscall calls from close_range()?


Thanks,
Corinna

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-15 12:07           ` Corinna Vinschen
@ 2024-01-15 12:08             ` Corinna Vinschen
  2024-01-15 12:41               ` Christian Franke
  0 siblings, 1 reply; 11+ messages in thread
From: Corinna Vinschen @ 2024-01-15 12:08 UTC (permalink / raw)
  To: cygwin-patches

On Jan 15 13:07, Corinna Vinschen wrote:
> Sorry Christian, but..
> 
> I was just going to push this patch when I realized that we now have
> two lines of debug output per affected file descriptor:
> 
> On Jan 15 12:19, Christian Franke wrote:
> > +  for (unsigned int i = firstfd; i < size; i++)
> > +    {
> > +      cygheap_fdget cfd ((int) i, false, false);
> > +      if (cfd < 0)
> > +	continue;
> > +
> > +      if (flags & CLOSE_RANGE_CLOEXEC)
> > +	{
> > +	  syscall_printf ("set FD_CLOEXEC on fd %u", i);
> > +	  cfd->fcntl (F_SETFD, FD_CLOEXEC);
> 
> fhandler::set_close_on_exec() already prints this:
> 
>   debug_printf ("set close_on_exec for %s to %d", get_name (), val);
> 
> > +	}
> > +      else
> > +	{
> > +	  syscall_printf ("closing fd %u", i);
> > +	  cfd->close_with_arch ();
> 
> fhandler::close() already prints this:
> 
>   syscall_printf ("closing '%s' handle %p", get_name (), get_handle ());
> 
> Shan't we drop the syscall calls from close_range()?
                     ^^^^^^^
                   syscall_printf

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-15 12:08             ` Corinna Vinschen
@ 2024-01-15 12:41               ` Christian Franke
  2024-01-15 12:56                 ` Corinna Vinschen
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Franke @ 2024-01-15 12:41 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1250 bytes --]

Corinna Vinschen wrote:
> On Jan 15 13:07, Corinna Vinschen wrote:
>> Sorry Christian, but..
>>
>> I was just going to push this patch when I realized that we now have
>> two lines of debug output per affected file descriptor:
>>
>> On Jan 15 12:19, Christian Franke wrote:
>>> +  for (unsigned int i = firstfd; i < size; i++)
>>> +    {
>>> +      cygheap_fdget cfd ((int) i, false, false);
>>> +      if (cfd < 0)
>>> +	continue;
>>> +
>>> +      if (flags & CLOSE_RANGE_CLOEXEC)
>>> +	{
>>> +	  syscall_printf ("set FD_CLOEXEC on fd %u", i);
>>> +	  cfd->fcntl (F_SETFD, FD_CLOEXEC);
>> fhandler::set_close_on_exec() already prints this:
>>
>>    debug_printf ("set close_on_exec for %s to %d", get_name (), val);
>>
>>> +	}
>>> +      else
>>> +	{
>>> +	  syscall_printf ("closing fd %u", i);
>>> +	  cfd->close_with_arch ();
>> fhandler::close() already prints this:
>>
>>    syscall_printf ("closing '%s' handle %p", get_name (), get_handle ());

I've also seen this duplication, but the drawback of the above messages 
is that the FD itself is not printed. So I decided to keep the 
syscall_printf().


>>
>> Shan't we drop the syscall calls from close_range()?
>                       ^^^^^^^
>                     syscall_printf

Attached.


[-- Attachment #2: 0001-Cygwin-introduce-close_range-2.patch --]
[-- Type: text/plain, Size: 6072 bytes --]

From ca97bb30210d3c90ae94f5ff29ddd75a12a8a8b1 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Mon, 15 Jan 2024 13:36:30 +0100
Subject: [PATCH] Cygwin: introduce close_range(2)

This function closes or sets the close-on-exec flag for a specified
range of file descriptors.  It is available on FreeBSD and Linux.

Signed-off-by: Christian Franke <christian.franke@t-online.de>
---
 newlib/libc/include/sys/unistd.h       |  6 ++++
 winsup/cygwin/cygwin.din               |  1 +
 winsup/cygwin/include/cygwin/version.h |  3 +-
 winsup/cygwin/release/3.5.0            |  2 ++
 winsup/cygwin/syscalls.cc              | 38 ++++++++++++++++++++++++++
 winsup/doc/new-features.xml            |  4 +++
 winsup/doc/posix.xml                   |  5 ++++
 7 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index 25532251c..00901540f 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -26,6 +26,12 @@ int     chown (const char *__path, uid_t __owner, gid_t __group);
 int     chroot (const char *__path);
 #endif
 int     close (int __fildes);
+#if defined(__CYGWIN__) && (__BSD_VISIBLE || __GNU_VISIBLE)
+/* Available on FreeBSD (__BSD_VISIBLE) and Linux (__GNU_VISIBLE). */
+int     close_range (unsigned int __firstfd, unsigned int __lastfd, int __flags);
+/*      CLOSE_RANGE_UNSHARE (1 << 1) */ /* Linux-specific, not supported. */
+#define CLOSE_RANGE_CLOEXEC (1 << 2)
+#endif
 #if __POSIX_VISIBLE >= 199209
 size_t	confstr (int __name, char *__buf, size_t __len);
 #endif
diff --git a/winsup/cygwin/cygwin.din b/winsup/cygwin/cygwin.din
index 9b76ce67a..9e354acc6 100644
--- a/winsup/cygwin/cygwin.din
+++ b/winsup/cygwin/cygwin.din
@@ -347,6 +347,7 @@ clog10l NOSIGFE
 clogf NOSIGFE
 clogl NOSIGFE
 close SIGFE
+close_range SIGFE
 closedir SIGFE
 closelog SIGFE
 cnd_broadcast SIGFE
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index c8177c2b1..3036878c4 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -484,12 +484,13 @@ details. */
   347: Add c16rtomb, c32rtomb, mbrtoc16, mbrtoc32.
   348: Add c8rtomb, mbrtoc.
   349: Add fallocate.
+  350: Add close_range.
 
   Note that we forgot to bump the api for ualarm, strtoll, strtoull,
   sigaltstack, sethostname. */
 
 #define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 349
+#define CYGWIN_VERSION_API_MINOR 350
 
 /* There is also a compatibity version number associated with the shared memory
    regions.  It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/release/3.5.0 b/winsup/cygwin/release/3.5.0
index d0a6c2fc8..6209064a6 100644
--- a/winsup/cygwin/release/3.5.0
+++ b/winsup/cygwin/release/3.5.0
@@ -43,6 +43,8 @@ What's new:
 
 - New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 
+- New API call: close_range (available on FreeBSD and Linux).
+
 - New API call: fallocate (Linux-specific).
 
 - Implement OSS-based sound mixer device (/dev/mixer).
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 486db1db6..eb04067fd 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -85,6 +85,44 @@ close_all_files (bool norelease)
   cygheap->fdtab.unlock ();
 }
 
+/* Close or set the close-on-exec flag for all open file descriptors
+   from firstfd to lastfd.  CLOSE_RANGE_UNSHARE is not supported.
+   Available on FreeBSD since 13 and Linux since 5.9 */
+extern "C" int
+close_range (unsigned int firstfd, unsigned int lastfd, int flags)
+{
+  pthread_testcancel ();
+
+  if (!(firstfd <= lastfd && !(flags & ~CLOSE_RANGE_CLOEXEC)))
+    {
+      set_errno (EINVAL);
+      return -1;
+    }
+
+  cygheap->fdtab.lock ();
+
+  unsigned int size = (lastfd < cygheap->fdtab.size ? lastfd + 1 :
+		      cygheap->fdtab.size);
+
+  for (unsigned int i = firstfd; i < size; i++)
+    {
+      cygheap_fdget cfd ((int) i, false, false);
+      if (cfd < 0)
+	continue;
+
+      if (flags & CLOSE_RANGE_CLOEXEC)
+	cfd->fcntl (F_SETFD, FD_CLOEXEC);
+      else
+	{
+	  cfd->close_with_arch ();
+	  cfd.release ();
+	}
+    }
+
+  cygheap->fdtab.unlock ();
+  return 0;
+}
+
 extern "C" int
 dup (int fd)
 {
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 6ae420031..0abe1c41c 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -74,6 +74,10 @@ posix_spawn_file_actions_addfchdir_np.
 New API calls: c8rtomb, c16rtomb, c32rtomb, mbrtoc8, mbrtoc16, mbrtoc32.
 </para></listitem>
 
+<listitem><para>
+New API call: close_range (available on FreeBSD and Linux).
+</para></listitem>
+
 <listitem><para>
 New API call: fallocate (Linux-specific).
 </para></listitem>
diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml
index 1a4eee1ab..89056915b 100644
--- a/winsup/doc/posix.xml
+++ b/winsup/doc/posix.xml
@@ -1143,6 +1143,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
     cfmakeraw
     cfsetspeed
     clearerr_unlocked
+    close_range
     daemon
     dn_comp
     dn_expand
@@ -1297,6 +1298,7 @@ also IEEE Std 1003.1-2017 (POSIX.1-2017).</para>
     clog10
     clog10f
     clog10l
+    close_range			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
     crypt_r			(available in external "crypt" library)
     dladdr			(see <xref linkend="std-notes">chapter "Implementation Notes"</xref>)
     dremf
@@ -1656,6 +1658,9 @@ CLOCK_REALTIME and CLOCK_MONOTONIC.  <function>clock_setres</function>,
 <function>clock_settime</function>, and <function>timer_create</function>
 currently support only CLOCK_REALTIME.</para>
 
+<para><function>close_range</function> does not support the Linux-specific
+flag CLOSE_RANGE_UNSHARE.</para>
+
 <para>POSIX file locks via <function>fcntl</function> or
 <function>lockf</function>, as well as BSD <function>flock</function> locks
 are advisory locks.  They don't interact with Windows mandatory locks, nor
-- 
2.42.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Cygwin: introduce close_range
  2024-01-15 12:41               ` Christian Franke
@ 2024-01-15 12:56                 ` Corinna Vinschen
  0 siblings, 0 replies; 11+ messages in thread
From: Corinna Vinschen @ 2024-01-15 12:56 UTC (permalink / raw)
  To: cygwin-patches

On Jan 15 13:41, Christian Franke wrote:
> Corinna Vinschen wrote:
> > On Jan 15 13:07, Corinna Vinschen wrote:
> > > Sorry Christian, but..
> > > 
> > > I was just going to push this patch when I realized that we now have
> > > two lines of debug output per affected file descriptor:
> > > 
> > > On Jan 15 12:19, Christian Franke wrote:
> > > > +  for (unsigned int i = firstfd; i < size; i++)
> > > > +    {
> > > > +      cygheap_fdget cfd ((int) i, false, false);
> > > > +      if (cfd < 0)
> > > > +	continue;
> > > > +
> > > > +      if (flags & CLOSE_RANGE_CLOEXEC)
> > > > +	{
> > > > +	  syscall_printf ("set FD_CLOEXEC on fd %u", i);
> > > > +	  cfd->fcntl (F_SETFD, FD_CLOEXEC);
> > > fhandler::set_close_on_exec() already prints this:
> > > 
> > >    debug_printf ("set close_on_exec for %s to %d", get_name (), val);
> > > 
> > > > +	}
> > > > +      else
> > > > +	{
> > > > +	  syscall_printf ("closing fd %u", i);
> > > > +	  cfd->close_with_arch ();
> > > fhandler::close() already prints this:
> > > 
> > >    syscall_printf ("closing '%s' handle %p", get_name (), get_handle ());
> 
> I've also seen this duplication, but the drawback of the above messages is
> that the FD itself is not printed.

In both cases, that's right.  Good point, actually.

> So I decided to keep the
> syscall_printf().

I pushed the patch now with the additional syscall_printf, FWIW.
If they really annoy people, which unlikely, we can still drop them.


Thanks,
Corinna

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-01-15 12:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-14 16:07 [PATCH] Cygwin: introduce close_range Christian Franke
2024-01-14 17:30 ` Jon Turney
2024-01-14 18:53   ` Christian Franke
2024-01-15  8:56     ` Christian Franke
2024-01-15 10:44       ` Corinna Vinschen
2024-01-15 11:19         ` Christian Franke
2024-01-15 12:07           ` Corinna Vinschen
2024-01-15 12:08             ` Corinna Vinschen
2024-01-15 12:41               ` Christian Franke
2024-01-15 12:56                 ` Corinna Vinschen
2024-01-15  9:50     ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).