Re: [PATCH] Cygwin: select: don't report read ready on a FIFO never, opened for writing
On 10/18/2022 11:45 AM, Corinna Vinschen wrote: On Sep 23 11:31, Ken Brown wrote: @@ -1043,6 +1043,8 @@ writer_shmem: set_pipe_non_blocking (get_handle (), flags & O_NONBLOCK); nwriters_lock (); inc_nwriters (); + if (!writer_opened () ) +set_writer_opened (); My personal preference would be to skip the writer_opened() check and just call set_writer_opened(), but that's just me. I agree. I've pushed it with that change. Ken
[PATCH] Cygwin: select: don't report read ready on a FIFO never, opened for writing
Patch attached. I'm also attaching a test case, that behaves the same on Cygwin
as on Linux.
Ken#include
#include
#include
#include
#include
#include
#include
#define FIFO_PATH "/tmp/myfifo"
int
main ()
{
int fd, tmpfd, nsel;
fd_set readfds;
struct timeval wait_tm = { 0l, 20l }; /* 200 millisecs */
if (unlink (FIFO_PATH) < 0 && errno != ENOENT)
{
perror ("unlink");
exit (1);
}
if (mkfifo (FIFO_PATH, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH
| S_IWOTH) < 0)
{
perror ("mkfifo");
exit (1);
}
printf ("Opening a FIFO for reading with O_NONBLOCK\n");
if ((fd = open (FIFO_PATH, O_RDONLY | O_NONBLOCK)) < 0)
{
perror ("open");
exit (1);
}
printf ("Testing for read ready...\n");
FD_ZERO ();
FD_SET (fd, );
nsel = select (fd + 1, , NULL, NULL, _tm);
printf (" select returned %d\n", nsel);
printf ("Opening and closing FIFO for writing...\n");
if ((tmpfd = open (FIFO_PATH, O_WRONLY)) < 0)
{
perror ("open");
exit (1);
}
if (close (tmpfd) < 0)
{
perror ("close");
exit (1);
}
FD_ZERO ();
FD_SET (fd, );
nsel = select (fd + 1, , NULL, NULL, _tm);
printf (" now select returned %d\n", nsel);
}
From f4a92734eac17dbfc7ff3541eef9611c87184ed0 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Fri, 23 Sep 2022 10:24:04 -0400
Subject: [PATCH] Cygwin: select: don't report read ready on a FIFO never
opened for writing
According to POSIX and the Linux man page, select(2) is supposed to
report read ready if a file is at EOF. In the case of a FIFO, this
means that the pipe is empty and there are no writers. But there
seems to be an undocumented exception, observed on Linux and other
platforms: If no writer has ever been opened, then select(2) does not
report read ready. This can happen if a reader is opened with
O_NONBLOCK before any writers have opened.
This commit makes Cygwin consistent with those other platforms by
introducing a special EOF test, fhandler_fifo::select_hit_eof, which
returns false if there's never been a writer opened.
To implement this we use a new variable '_writer_opened' in the FIFO's
shared memory, which is set to 1 the first time a writer opens. New
methods writer_opened() and set_writer_opened() are used to test and
set this variable.
Addresses: https://cygwin.com/pipermail/cygwin/2022-September/252223.html
---
winsup/cygwin/fhandler/fifo.cc | 2 ++
winsup/cygwin/local_includes/fhandler.h | 9 +
winsup/cygwin/release/3.3.4 | 3 +++
winsup/cygwin/select.cc | 12 +++-
4 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler/fifo.cc b/winsup/cygwin/fhandler/fifo.cc
index 1d3e42908..ecfb33bff 100644
--- a/winsup/cygwin/fhandler/fifo.cc
+++ b/winsup/cygwin/fhandler/fifo.cc
@@ -1043,6 +1043,8 @@ writer_shmem:
set_pipe_non_blocking (get_handle (), flags & O_NONBLOCK);
nwriters_lock ();
inc_nwriters ();
+ if (!writer_opened () )
+set_writer_opened ();
SetEvent (write_ready);
ResetEvent (writer_opening);
nwriters_unlock ();
diff --git a/winsup/cygwin/local_includes/fhandler.h
b/winsup/cygwin/local_includes/fhandler.h
index aad7f4c37..b012c6e8f 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -1327,6 +1327,8 @@ struct fifo_reader_id_t
class fifo_shmem_t
{
LONG _nreaders, _nwriters;
+ /* Set to 1 the first time a writer opens. */
+ LONG _writer_opened;
fifo_reader_id_t _owner, _prev_owner, _pending_owner;
af_unix_spinlock_t _owner_lock, _reading_lock, _nreaders_lock,
_nwriters_lock;
@@ -1343,6 +1345,9 @@ public:
int inc_nwriters () { return (int) InterlockedIncrement (&_nwriters); }
int dec_nwriters () { return (int) InterlockedDecrement (&_nwriters); }
+ bool writer_opened () const { return (bool) _writer_opened; }
+ void set_writer_opened () { InterlockedExchange (&_writer_opened, 1); }
+
fifo_reader_id_t get_owner () const { return _owner; }
void set_owner (fifo_reader_id_t fr_id) { _owner = fr_id; }
fifo_reader_id_t get_prev_owner () const { return _prev_owner; }
@@ -1425,6 +1430,8 @@ class fhandler_fifo: public fhandler_pipe_fifo
int nwriters () const { return shmem->nwriters (); }
int inc_nwriters () { return shmem->inc_nwriters (); }
int dec_nwriters () { return shmem->dec_nwriters (); }
+ bool writer_opened () const { return shmem->writer_opened (); }
+ void set_writer_opened () { shmem->set_writer_opened (); }
void nreaders_lock () { shmem->nreaders_lock (); }
void nreaders_unlock () { shmem->nreaders_unlock (); }
void nwriters_lock () { shmem->nwriters_lock (); }
@@ -1480,6 +1487,8 @@ public:
/* Called if we appear to be at EOF
Re: [PATCH] Cygwin: fix return value of symlink_info::check
v2 attached with a more accurate commit message.
Ken
On 8/9/2022 3:52 PM, Ken Brown wrote:
Patch attached. Please check my changes to the commentary preceding
symlink_info::check to make sure I got it right.
I've written the patch against the master branch, but I think it should be
applied to cygwin-3_3-branch also.
KenFrom d1aee2f7e022497d57d55ffcd69ddaa7d7b123b2 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 9 Aug 2022 15:14:07 -0400
Subject: [PATCH v2] Cygwin: fix return value of symlink_info::check
Currently it is possible for symlink_info::check to return -1 in case
we're searching for foo and find foo.lnk that is not a Cygwin symlink.
This contradicts the new meaning attached to a negative return value
in commit 19d59ce75d. Fix this by setting "res" to 0 at the beginning
of the main loop and not seting it to -1 later.
Also fix the commentary preceding the function definition to reflect
the current behavior.
Addresses: https://cygwin.com/pipermail/cygwin/2022-August/252030.html
---
winsup/cygwin/path.cc | 22 +-
winsup/cygwin/release/3.3.6 | 4
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 3e436dc65..227b99d0f 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3027,19 +3027,16 @@ symlink_info::parse_device (const char *contents)
/* Check if PATH is a symlink. PATH must be a valid Win32 path name.
If PATH is a symlink, put the value of the symlink--the file to
- which it points--into BUF. The value stored in BUF is not
- necessarily null terminated. BUFLEN is the length of BUF; only up
- to BUFLEN characters will be stored in BUF. BUF may be NULL, in
- which case nothing will be stored.
+ which it points--into CONTENTS.
- Set *SYML if PATH is a symlink.
+ Set PATH_SYMLINK if PATH is a symlink.
- Set *EXEC if PATH appears to be executable. This is an efficiency
- hack because we sometimes have to open the file anyhow. *EXEC will
- not be set for every executable file.
-
- Return -1 on error, 0 if PATH is not a symlink, or the length
- stored into BUF if PATH is a symlink. */
+ If PATH is a symlink, return the length stored into CONTENTS. If
+ the inner components of PATH contain native symlinks or junctions,
+ or if the drive is a virtual drive, compare PATH with the result
+ returned by GetFinalPathNameByHandleA. If they differ, store the
+ final path in CONTENTS and return the negative of its length. In
+ all other cases, return 0. */
int
symlink_info::check (char *path, const suffix_info *suffixes, fs_info ,
@@ -3094,6 +3091,7 @@ restart:
while (suffix.next ())
{
+ res = 0;
error = 0;
get_nt_native_path (suffix.path, upath, mount_flags & MOUNT_DOS);
if (h)
@@ -3345,8 +3343,6 @@ restart:
continue;
}
- res = -1;
-
/* Reparse points are potentially symlinks. This check must be
performed before checking the SYSTEM attribute for sysfile
symlinks, since reparse points can have this flag set, too. */
diff --git a/winsup/cygwin/release/3.3.6 b/winsup/cygwin/release/3.3.6
index 078e6e520..364e0cb0d 100644
--- a/winsup/cygwin/release/3.3.6
+++ b/winsup/cygwin/release/3.3.6
@@ -35,3 +35,7 @@ Bug Fixes
- Fix a problem that prevented some symbolic links to /cygdrive/C,
/cygdrive/./c, /cygdrive//c, etc. from working.
Addresses: https://cygwin.com/pipermail/cygwin/2022-July/251994.html
+
+- Fix a path handling bug that could cause a non-existing file to be
+ treated as the current directory.
+ Addresses: https://cygwin.com/pipermail/cygwin/2022-August/252030.html
--
2.37.1
[PATCH] Cygwin: fix return value of symlink_info::check
Patch attached. Please check my changes to the commentary preceding
symlink_info::check to make sure I got it right.
I've written the patch against the master branch, but I think it should be
applied to cygwin-3_3-branch also.
KenFrom fed0180d59d8102e0c671537a360abb12e8204a0 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 9 Aug 2022 15:14:07 -0400
Subject: [PATCH] Cygwin: fix return value of symlink_info::check
Currently it is possible for symlink_info::check to return -1 in case
we're searching for foo and find foo.lnk that is not a shortcut. This
contradicts the new meaning attached to a negative return value in
commit 19d59ce75d. Fix this by setting "res" to 0 at the beginning of
the main loop and not seting it to -1 later.
Also fix the commentary preceding the function definition to reflect
the current behavior.
Addresses: https://cygwin.com/pipermail/cygwin/2022-August/252030.html
---
winsup/cygwin/path.cc | 22 +-
winsup/cygwin/release/3.3.6 | 4
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 3e436dc65..227b99d0f 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3027,19 +3027,16 @@ symlink_info::parse_device (const char *contents)
/* Check if PATH is a symlink. PATH must be a valid Win32 path name.
If PATH is a symlink, put the value of the symlink--the file to
- which it points--into BUF. The value stored in BUF is not
- necessarily null terminated. BUFLEN is the length of BUF; only up
- to BUFLEN characters will be stored in BUF. BUF may be NULL, in
- which case nothing will be stored.
+ which it points--into CONTENTS.
- Set *SYML if PATH is a symlink.
+ Set PATH_SYMLINK if PATH is a symlink.
- Set *EXEC if PATH appears to be executable. This is an efficiency
- hack because we sometimes have to open the file anyhow. *EXEC will
- not be set for every executable file.
-
- Return -1 on error, 0 if PATH is not a symlink, or the length
- stored into BUF if PATH is a symlink. */
+ If PATH is a symlink, return the length stored into CONTENTS. If
+ the inner components of PATH contain native symlinks or junctions,
+ or if the drive is a virtual drive, compare PATH with the result
+ returned by GetFinalPathNameByHandleA. If they differ, store the
+ final path in CONTENTS and return the negative of its length. In
+ all other cases, return 0. */
int
symlink_info::check (char *path, const suffix_info *suffixes, fs_info ,
@@ -3094,6 +3091,7 @@ restart:
while (suffix.next ())
{
+ res = 0;
error = 0;
get_nt_native_path (suffix.path, upath, mount_flags & MOUNT_DOS);
if (h)
@@ -3345,8 +3343,6 @@ restart:
continue;
}
- res = -1;
-
/* Reparse points are potentially symlinks. This check must be
performed before checking the SYSTEM attribute for sysfile
symlinks, since reparse points can have this flag set, too. */
diff --git a/winsup/cygwin/release/3.3.6 b/winsup/cygwin/release/3.3.6
index 078e6e520..364e0cb0d 100644
--- a/winsup/cygwin/release/3.3.6
+++ b/winsup/cygwin/release/3.3.6
@@ -35,3 +35,7 @@ Bug Fixes
- Fix a problem that prevented some symbolic links to /cygdrive/C,
/cygdrive/./c, /cygdrive//c, etc. from working.
Addresses: https://cygwin.com/pipermail/cygwin/2022-July/251994.html
+
+- Fix a path handling bug that could cause a non-existing file to be
+ treated as the current directory.
+ Addresses: https://cygwin.com/pipermail/cygwin/2022-August/252030.html
--
2.37.1
[PATCH] Cygwin: syscalls.cc: remove ".dll" from, blessed_executable_suffixes
Patch attached. I'm not 100% sure of this since it does change behavior (but in
a weird case).
KenFrom 97a2fc0d07c8f9045b73716ac5a05f972d5bd75c Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Wed, 3 Aug 2022 16:45:23 -0400
Subject: [PATCH] Cygwin: syscalls.cc: remove ".dll" from
blessed_executable_suffixes
This reverts commit d9e9c7b5a7. The latter added ".dll" to the
blessed_executable_suffixes array because on 32-bit Windows, the
GetBinaryType function would report that a 64-bit DLL is an
executable, contrary to the documentation of that function.
That anomaly does not exist on 64-bit Windows, so we can remove ".dll"
from the list. Reverting the commit does, however, change the
behavior of the rename(2) syscall in the following unlikely situation:
Suppose we have an executable foo.exe and we make the call
rename ("foo", "bar.dll");
Previously, foo.exe would be renamed to bar.dll. So bar.dll would
then be an executable without the .exe extension. The new behavior is
that foo.exe will be renamed to bar.dll.exe. [Exception: If there
already existed an executable (not a DLL!) with the name bar.dll, then
.exe will not be appended.]
---
winsup/cygwin/globals.cc | 1 -
winsup/cygwin/syscalls.cc | 6 --
2 files changed, 7 deletions(-)
diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index e8147cb5c..e909d0f8f 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -120,7 +120,6 @@ const int __collate_load_error = 0;
extern UNICODE_STRING _RDATA ro_u_empty = _ROU (L"");
extern UNICODE_STRING _RDATA ro_u_lnk = _ROU (L".lnk");
extern UNICODE_STRING _RDATA ro_u_exe = _ROU (L".exe");
- extern UNICODE_STRING _RDATA ro_u_dll = _ROU (L".dll");
extern UNICODE_STRING _RDATA ro_u_com = _ROU (L".com");
extern UNICODE_STRING _RDATA ro_u_scr = _ROU (L".scr");
extern UNICODE_STRING _RDATA ro_u_sys = _ROU (L".sys");
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index e6d68cc07..599b2b793 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2133,12 +2133,6 @@ nt_path_has_executable_suffix (PUNICODE_STRING upath)
static const PUNICODE_STRING blessed_executable_suffixes[] =
{
_u_com,
-_u_dll, /* Messy, messy. Per MSDN, the GetBinaryType function is
- supposed to return with ERROR_BAD_EXE_FORMAT. if the file
- is a DLL. On 64-bit Windows, this works as expected for
- 32-bit and 64-bit DLLs. On 32-bit Windows this only works
- for 32-bit DLLs. For 64-bit DLLs, 32-bit Windows returns
- true with the type set to SCS_64BIT_BINARY. */
_u_exe,
_u_scr,
_u_sys,
--
2.37.1
Re: [PATCH] Cygwin: redefine some macros for Linux compatibility
On 7/6/2022 4:25 PM, Ken Brown wrote: Patch attached. I wasn't sure whether the API bump was warranted for such a trivial change. But the fact is that some programs compiled prior to the patch will behave differently if they are recompiled after the patch. For example, emacs limits the number of open subprocesses to FD_SETSIZE, so this number will change when emacs is recompiled for Cygwin 3.4.0. Is that a good enough reason to bump the API? Oh, wait a minute there's already been an API bump from 341 to 342, so I guess there's no need for another. I could just add the FD_SETSIZE and NOFILE changes to the explanation for that bump. Ken
[PATCH] Cygwin: redefine some macros for Linux compatibility
Patch attached. I wasn't sure whether the API bump was warranted for such a trivial change. But the fact is that some programs compiled prior to the patch will behave differently if they are recompiled after the patch. For example, emacs limits the number of open subprocesses to FD_SETSIZE, so this number will change when emacs is recompiled for Cygwin 3.4.0. Is that a good enough reason to bump the API? KenFrom 6d6c1ed356e3e8f39aa9b6447982f3fccbdabfad Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Wed, 6 Jul 2022 14:43:16 -0400 Subject: [PATCH] Cygwin: redefine some macros for Linux compatibility Define FD_SETSIZE () to be 1024 by default, and define NOFILE () to be OPEN_MAX (== 3200) by default. Remove the comment in that FD_SETSIZE should be >= NOFILE. Bump API minor. Addresses: https://cygwin.com/pipermail/cygwin/2022-July/251839.html --- newlib/libc/include/sys/select.h | 10 +++--- winsup/cygwin/include/cygwin/version.h | 3 ++- winsup/cygwin/include/sys/param.h | 4 +++- winsup/cygwin/release/3.4.0| 4 winsup/doc/new-features.xml| 5 + 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/newlib/libc/include/sys/select.h b/newlib/libc/include/sys/select.h index a5cd6c3fe..93d0b79bf 100644 --- a/newlib/libc/include/sys/select.h +++ b/newlib/libc/include/sys/select.h @@ -25,10 +25,14 @@ typedef __sigset_t sigset_t; * Select uses bit masks of file descriptors in longs. * These macros manipulate such bit fields (the filesystem macros use chars). * FD_SETSIZE may be defined by the user, but the default here - * should be >= NOFILE (param.h). + * should be enough for most uses. */ -#ifndefFD_SETSIZE -#defineFD_SETSIZE 64 +#ifndef FD_SETSIZE +# ifdef __CYGWIN__ +# define FD_SETSIZE 1024 +# else +# define FD_SETSIZE 64 +# endif #endif typedef unsigned long __fd_mask; diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h index 6f65a1299..a5d38f37a 100644 --- a/winsup/cygwin/include/cygwin/version.h +++ b/winsup/cygwin/include/cygwin/version.h @@ -517,12 +517,13 @@ details. */ pthread_rwlock_clockrdlock, pthread_rwlock_clockwrlock, sem_clockwait, sig2str, str2sig. 342: Remove cleanup_glue. + 343: Change FD_SETSIZE and NOFILE. 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 342 +#define CYGWIN_VERSION_API_MINOR 343 /* 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/include/sys/param.h b/winsup/cygwin/include/sys/param.h index 63de726e6..742599b8b 100644 --- a/winsup/cygwin/include/sys/param.h +++ b/winsup/cygwin/include/sys/param.h @@ -17,7 +17,9 @@ /* Max number of open files. The Posix version is OPEN_MAX. */ /* Number of fds is virtually unlimited in cygwin, but we must provide some reasonable value for Posix conformance */ -#define NOFILE 8192 +#if !defined NOFILE && defined OPEN_MAX +# define NOFILE OPEN_MAX +#endif /* Max number of groups; must keep in sync with NGROUPS_MAX in limits.h */ #define NGROUPSNGROUPS_MAX diff --git a/winsup/cygwin/release/3.4.0 b/winsup/cygwin/release/3.4.0 index f310912c9..08d28d510 100644 --- a/winsup/cygwin/release/3.4.0 +++ b/winsup/cygwin/release/3.4.0 @@ -25,6 +25,10 @@ What changed: the current directory as Linux does. Addresses: https://cygwin.com/pipermail/cygwin/2022-June/251730.html +- The default values of FD_SETSIZE and NOFILE are now 1024 and 3200, + respectively. + Addresses: https://cygwin.com/pipermail/cygwin/2022-July/251839.html + Bug Fixes - diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml index 0149a027a..46dc028f1 100644 --- a/winsup/doc/new-features.xml +++ b/winsup/doc/new-features.xml @@ -33,6 +33,11 @@ Handle UDP_SEGMENT and UDP_GRO socket options. The stdio input functions no longer try again to read after EOF. + +The default values of FD_SETSIZE and NOFILE are now 1024 and 3200, +respectively. + + -- 2.36.1
Re: [PATCH] Cygwin: spawn: Treat empty path as the current directory.
On 7/4/2022 4:37 AM, Corinna Vinschen wrote: On Jul 1 21:32, Ken Brown wrote: I interpreted the existing comment as meaning that a decision was already made at some point to align with Linux. But it can't hurt to wait for Corinna to weigh in. Personally I don't like this old crufty feature and I would rather keep this POSIX compatible, but in fact it was meant to work as on Linux, so, please go ahead, Takashi. However, maybe this should go into the master branch only? WDYT? I think so. The bug has been around for a long time, and the code is tricky. So we probably shouldn't take a chance on de-stabilizing the 3.3 branch. Ken
[PATCH] Cygwin: implement getfacl(1) for socket files
Patch attached.From 28cd29d99fe6d1a54c8dad04854bda10743737d3 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sat, 2 Jul 2022 15:12:40 -0400
Subject: [PATCH] Cygwin: implement getfacl(1) for socket files
Do this by defining the acl_get method for the fhandler_socket_local
and fhandler_socket_unix classes. Also define acl_set for these
classes.
Partially addresses: https://cygwin.com/pipermail/cygwin/2022-July/251768.html
---
winsup/cygwin/fhandler.h | 4 ++
winsup/cygwin/release/3.3.6 | 3 ++
winsup/cygwin/sec_posixacl.cc | 76 +++
3 files changed, 83 insertions(+)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index d5ec56a6d..cb5e08fa3 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -861,6 +861,8 @@ class fhandler_socket_local: public fhandler_socket_wsock
int fchmod (mode_t newmode);
int fchown (uid_t newuid, gid_t newgid);
int facl (int, int, struct acl *);
+ struct __acl_t *acl_get (uint32_t);
+ int acl_set (struct __acl_t *, uint32_t);
int link (const char *);
/* from here on: CLONING */
@@ -1143,6 +1145,8 @@ class fhandler_socket_unix : public fhandler_socket
int fchmod (mode_t newmode);
int fchown (uid_t newuid, gid_t newgid);
int facl (int, int, struct acl *);
+ struct __acl_t *acl_get (uint32_t);
+ int acl_set (struct __acl_t *, uint32_t);
int link (const char *);
/* select.cc */
diff --git a/winsup/cygwin/release/3.3.6 b/winsup/cygwin/release/3.3.6
index 44a7bcf9d..b18691428 100644
--- a/winsup/cygwin/release/3.3.6
+++ b/winsup/cygwin/release/3.3.6
@@ -26,3 +26,6 @@ Bug Fixes
- Fix a console problem that the text longer than 1024 bytes cannot
be pasted correctly.
Addresses: https://cygwin.com/pipermail/cygwin/2022-June/251764.html
+
+- Don't error out if getfacl(1) is called on a socket file.
+ Partially addresses:
https://cygwin.com/pipermail/cygwin/2022-July/251768.html
diff --git a/winsup/cygwin/sec_posixacl.cc b/winsup/cygwin/sec_posixacl.cc
index e7e5a9c3e..c2daa3309 100644
--- a/winsup/cygwin/sec_posixacl.cc
+++ b/winsup/cygwin/sec_posixacl.cc
@@ -633,6 +633,44 @@ fhandler_disk_file::acl_get (acl_type_t type)
return acl;
}
+acl_t
+fhandler_socket_local::acl_get (acl_type_t type)
+{
+ if (!dev ().isfs ())
+/* acl_get_fd on a socket. */
+return fhandler_base::acl_get (type);
+
+ /* acl_get_fd on a socket opened with O_PATH or acl_get_file on a
+ socket file. */
+ if (get_flags () & O_PATH)
+{
+ set_errno (EBADF);
+ return NULL;
+}
+ fhandler_disk_file fh (pc);
+ return fh.acl_get (type);
+}
+
+#ifdef __WITH_AF_UNIX
+acl_t
+fhandler_socket_unix::acl_get (acl_type_t type)
+{
+ if (!dev ().isfs ())
+/* acl_get_fd on a socket. */
+return fhandler_base::acl_get (type);
+
+ /* acl_get_fd on a socket opened with O_PATH or acl_get_file on a
+ socket file. */
+ if (get_flags () & O_PATH)
+{
+ set_errno (EBADF);
+ return NULL;
+}
+ fhandler_disk_file fh (pc);
+ return fh.acl_get (type);
+}
+#endif
+
extern "C" acl_t
acl_get_fd (int fd)
{
@@ -765,6 +803,44 @@ fhandler_disk_file::acl_set (acl_t acl, acl_type_t type)
return ret;
}
+int
+fhandler_socket_local::acl_set (acl_t acl, acl_type_t type)
+{
+ if (!dev ().isfs ())
+/* acl_set_fd on a socket. */
+return fhandler_base::acl_set (acl, type);
+
+ /* acl_set_fd on a socket opened with O_PATH or acl_set_file on a
+ socket file. */
+ if (get_flags () & O_PATH)
+{
+ set_errno (EBADF);
+ return -1;
+}
+ fhandler_disk_file fh (pc);
+ return fh.acl_set (acl, type);
+}
+
+#ifdef __WITH_AF_UNIX
+int
+fhandler_socket_unix::acl_set (acl_t acl, acl_type_t type)
+{
+ if (!dev ().isfs ())
+/* acl_set_fd on a socket. */
+return fhandler_base::acl_set (acl, type);
+
+ /* acl_set_fd on a socket opened with O_PATH or acl_set_file on a
+ socket file. */
+ if (get_flags () & O_PATH)
+{
+ set_errno (EBADF);
+ return -1;
+}
+ fhandler_disk_file fh (pc);
+ return fh.acl_set (acl, type);
+}
+#endif
+
extern "C" int
acl_set_fd (int fd, acl_t acl)
{
--
2.36.1
Re: [PATCH] Cygwin: spawn: Treat empty path as the current directory.
On 7/1/2022 7:31 PM, Takashi Yano wrote: On Thu, 30 Jun 2022 21:16:35 -0400 Ken Brown wrote: On 6/30/2022 11:45 AM, Ken Brown wrote: On 6/27/2022 8:44 AM, Takashi Yano wrote: - With this patch, the empty path (empty element in PATH or PATH is absent) is treated as the current directory as Linux does. Addresses: https://cygwin.com/pipermail/cygwin/2022-June/251730.html It might be a good idea to include a comment in the code and the commit message that this feature is being added for Linux compatibility but that it is deprecated. According to https://man7.org/linux/man-pages/man7/environ.7.html, As a legacy feature, a zero-length prefix (specified as two adjacent colons, or an initial or terminating colon) is interpreted to mean the current working directory. However, use of this feature is deprecated, and POSIX notes that a conforming application shall use an explicit pathname (e.g., .) to specify the current working directory. Alternatively, maybe this is a case where we should prefer POSIX compliance to Linux compatibility. Corinna, WDYT? I withdraw my suggestion. There's already a comment in the code saying, "An empty path or '.' means the current directory", so it's clear that the intention was to support that feature, and the code was simply buggy. I've now read through the patch, and it looks good to me. This was pretty tricky to get right. We still need to discuss whether it is better to align Linux behavior or just keeping POSIX compliance, don't we? I interpreted the existing comment as meaning that a decision was already made at some point to align with Linux. But it can't hurt to wait for Corinna to weigh in. Ken
Re: [PATCH] Cygwin: spawn: Treat empty path as the current directory.
On 6/30/2022 11:45 AM, Ken Brown wrote: On 6/27/2022 8:44 AM, Takashi Yano wrote: - With this patch, the empty path (empty element in PATH or PATH is absent) is treated as the current directory as Linux does. Addresses: https://cygwin.com/pipermail/cygwin/2022-June/251730.html It might be a good idea to include a comment in the code and the commit message that this feature is being added for Linux compatibility but that it is deprecated. According to https://man7.org/linux/man-pages/man7/environ.7.html, As a legacy feature, a zero-length prefix (specified as two adjacent colons, or an initial or terminating colon) is interpreted to mean the current working directory. However, use of this feature is deprecated, and POSIX notes that a conforming application shall use an explicit pathname (e.g., .) to specify the current working directory. Alternatively, maybe this is a case where we should prefer POSIX compliance to Linux compatibility. Corinna, WDYT? I withdraw my suggestion. There's already a comment in the code saying, "An empty path or '.' means the current directory", so it's clear that the intention was to support that feature, and the code was simply buggy. I've now read through the patch, and it looks good to me. This was pretty tricky to get right. Ken
Re: [PATCH] Cygwin: poll: Fix a bug on inquiring same fd with different events.
On 6/26/2022 9:50 PM, Takashi Yano wrote: - poll() has a bug that it returns event which is not inquired if events are inquired in multiple pollfd entries on the same fd at the same time. This patch fixes the issue. Addresses: https://cygwin.com/pipermail/cygwin/2022-June/251732.html LGTM (but I haven't tested it). Ken
Re: [PATCH] Cygwin: spawn: Treat empty path as the current directory.
On 6/27/2022 8:44 AM, Takashi Yano wrote: - With this patch, the empty path (empty element in PATH or PATH is absent) is treated as the current directory as Linux does. Addresses: https://cygwin.com/pipermail/cygwin/2022-June/251730.html It might be a good idea to include a comment in the code and the commit message that this feature is being added for Linux compatibility but that it is deprecated. According to https://man7.org/linux/man-pages/man7/environ.7.html, As a legacy feature, a zero-length prefix (specified as two adjacent colons, or an initial or terminating colon) is interpreted to mean the current working directory. However, use of this feature is deprecated, and POSIX notes that a conforming application shall use an explicit pathname (e.g., .) to specify the current working directory. Alternatively, maybe this is a case where we should prefer POSIX compliance to Linux compatibility. Corinna, WDYT? Ken
Re: [PATCH 7/7] Cygwin: remove miscellaneous 32-bit code
On 6/11/2022 8:29 AM, Takashi Yano wrote: On Thu, 9 Jun 2022 15:04:29 -0400 Ken Brown wrote: OK, I'll make that change. Isn't the _dll_crt0() code in dcrt0.cc also x86_64 specific? Thanks for catching that. Should be fixed now. Ken
Re: [PATCH 7/7] Cygwin: remove miscellaneous 32-bit code
On 6/9/2022 12:00 PM, Yaakov Selkowitz wrote: On Thu, 2022-06-09 at 17:23 +0200, Corinna Vinschen wrote: On May 29 17:26, Ken Brown wrote: On 5/29/2022 9:39 AM, Jon Turney wrote: On 26/05/2022 20:17, Ken Brown wrote: winsup/cygwin/autoload.cc | 136 - -- Looks good. I think that perhaps the stdcall decoration number n is unused on x86_64, so can be removed also in a followup? Thanks, I missed that. Also, I guess most or all of the uses of __stdcall and __cdecl can be removed from the code. Yes, that's right, given there's only one calling convention on 64 bit. I have a minor objection in terms of this patch. When implementing support for AMD64, there were basically 2 problems to solve. One of them was to support 64 bit systems, the other one was to support AMD64. At that time, only IA-64 and AMD64 64 bit systems existed, and since we never considered IA-64 to run Cygwin on, we subsumed all 64 bit code paths under the __x86_64__ macro. But should we *ever* support ARM64, as unlikely as it is, we have to make sure to find all the places where the code is specificially AMD64. That goes, for instance, for all places calling assembler code, or for exception handling accessing CPU registers, etc. I'm open to discussion, but I think the code being CPU-specific should still be enclosed into #ifdef __x86_64__ brackets, with an #else #error alternative. Right? Wrong? Useless complication? Highly recommended. OK, I'll make that change. Ken
Re: [PATCH] Cygwin: remove most occurrences of __stdcall, WINAPI, and, __cdecl
On 6/7/2022 12:49 PM, Takashi Yano wrote: _dll_crt0() is declared as extern void __stdcall _dll_crt0 () __declspec (dllimport) __attribute__ ((noreturn)); in winsup/cygwin/lib/cygwin_crt0.c, however, this patch removes __stdcall from winsup.h and dcrt0.cc as follows. [...] To be consistent these, shouldn't _dll_crt0() retain __stdcall? Changing cygwin_crt0.c is a bit weird because it looks as if it might affect binary compatibility, even if it really doesn't. Thanks for catching this. It was actually an accident that I didn't remove __stdcall from cygwin_crt0.c, but I agree that it's better to retain it in the other two places. I've fixed this. Ken
Re: [PATCH] Cygwin: remove most occurrences of __stdcall, WINAPI, and, __cdecl
On 6/5/2022 4:24 PM, Jon Turney wrote: On 03/06/2022 15:00, Ken Brown wrote: remove most occurrences of __stdcall, WINAPI, and __cdecl These have no effect on x86_64. Retain only a few occurrences of __cdecl in files imported from other sources. While you are correct that it has no effect on x86_64, I'd incline towards retaining WINAPI on Windows API functions, because it's part of the function signature. But other people might have other opinions on that... I ended up retaining all occurrences of WINAPI. Those that don't directly occur in Windows API functions are mostly used for thread functions passed to CreateThread, and the latter expects a WINAPI function. Ken
Re: [PATCH] Cygwin: remove most occurrences of __stdcall, WINAPI, and, __cdecl
On 6/5/2022 4:24 PM, Jon Turney wrote: On 03/06/2022 15:00, Ken Brown wrote: remove most occurrences of __stdcall, WINAPI, and __cdecl These have no effect on x86_64. Retain only a few occurrences of __cdecl in files imported from other sources. While you are correct that it has no effect on x86_64, I'd incline towards retaining WINAPI on Windows API functions, because it's part of the function signature. But other people might have other opinions on that... I agree. I just mindlessly deleted all of them. I'll submit a revised patch. Ken
[PATCH] Cygwin: remove most occurrences of __stdcall, WINAPI, and, __cdecl
Patch attached.From f3e156ec3b22e426fc79138270f6e15c8156eb7b Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Sun, 29 May 2022 18:59:55 -0400 Subject: [PATCH] Cygwin: remove most occurrences of __stdcall, WINAPI, and __cdecl These have no effect on x86_64. Retain only a few occurrences of __cdecl in files imported from other sources. --- winsup/cygserver/bsd_helper.cc| 2 +- winsup/cygserver/process.cc | 2 +- winsup/cygserver/threaded_queue.cc| 4 +- winsup/cygserver/threaded_queue.h | 4 +- winsup/cygserver/woutsup.h| 2 +- winsup/cygwin/advapi32.cc | 8 ++-- winsup/cygwin/aio.cc | 6 +-- winsup/cygwin/autoload.cc | 4 +- winsup/cygwin/child_info.h| 2 +- winsup/cygwin/clock.cc| 4 +- winsup/cygwin/cygerrno.h | 2 +- winsup/cygwin/cygheap.cc | 8 ++-- winsup/cygwin/cygheap.h | 4 +- winsup/cygwin/cygthread.cc| 4 +- winsup/cygwin/cygthread.h | 6 +-- winsup/cygwin/dcrt0.cc| 12 +++--- winsup/cygwin/debug.cc| 10 ++--- winsup/cygwin/dll_init.cc | 2 +- winsup/cygwin/dtable.cc | 2 +- winsup/cygwin/environ.cc | 6 +-- winsup/cygwin/exceptions.cc | 14 +++--- winsup/cygwin/fhandler.cc | 10 ++--- winsup/cygwin/fhandler.h | 52 +++ winsup/cygwin/fhandler_clipboard.cc | 2 +- winsup/cygwin/fhandler_console.cc | 12 +++--- winsup/cygwin/fhandler_disk_file.cc | 2 +- winsup/cygwin/fhandler_dsp.cc | 4 +- winsup/cygwin/fhandler_fifo.cc| 2 +- winsup/cygwin/fhandler_netdrive.cc| 2 +- winsup/cygwin/fhandler_procsys.cc | 2 +- winsup/cygwin/fhandler_random.cc | 2 +- winsup/cygwin/fhandler_signalfd.cc| 2 +- winsup/cygwin/fhandler_socket_unix.cc | 8 ++-- winsup/cygwin/fhandler_timerfd.cc | 2 +- winsup/cygwin/fhandler_tty.cc | 12 +++--- winsup/cygwin/fhandler_virtual.cc | 2 +- winsup/cygwin/fhandler_windows.cc | 2 +- winsup/cygwin/fhandler_zero.cc| 2 +- winsup/cygwin/flock.cc| 6 +-- winsup/cygwin/fork.cc | 10 ++--- winsup/cygwin/include/cygwin/cygwin_dll.h | 6 +-- winsup/cygwin/include/cygwin/time.h | 6 +-- winsup/cygwin/include/sys/ioctl.h | 2 +- winsup/cygwin/init.cc | 4 +- winsup/cygwin/kernel32.cc | 36 winsup/cygwin/ldap.cc | 6 +-- winsup/cygwin/lib/_cygwin_crt0_common.cc | 2 +- winsup/cygwin/lib/crt0.h | 2 +- winsup/cygwin/miscfuncs.cc| 10 ++--- winsup/cygwin/miscfuncs.h | 10 ++--- winsup/cygwin/mmap.cc | 2 +- winsup/cygwin/mount.cc| 4 +- winsup/cygwin/ntdll.h | 2 +- winsup/cygwin/ntea.cc | 4 +- winsup/cygwin/ntsecapi.h | 26 winsup/cygwin/pinfo.cc| 6 +-- winsup/cygwin/pinfo.h | 12 +++--- winsup/cygwin/posix_timer.cc | 2 +- winsup/cygwin/resource.cc | 4 +- winsup/cygwin/select.cc | 10 ++--- winsup/cygwin/shared.cc | 10 ++--- winsup/cygwin/shared_info.h | 10 ++--- winsup/cygwin/shm.cc | 2 +- winsup/cygwin/sigproc.cc | 6 +-- winsup/cygwin/sigproc.h | 2 +- winsup/cygwin/strace.cc | 2 +- winsup/cygwin/strfuncs.cc | 10 ++--- winsup/cygwin/string.h| 8 ++-- winsup/cygwin/thread.cc | 2 +- winsup/cygwin/thread.h| 2 +- winsup/cygwin/timerfd.cc | 2 +- winsup/cygwin/times.cc| 16 +++ winsup/cygwin/tty.cc | 4 +- winsup/cygwin/tty.h | 6 +-- winsup/cygwin/window.cc | 4 +- winsup/cygwin/wininfo.h | 2 +- winsup/cygwin/winlean.h | 2 +- winsup/cygwin/winsup.h| 20 - winsup/testsuite/winsup.api/cygload.cc| 4 +- winsup/testsuite/winsup.api/cygload.h | 2 +- winsup/utils/kill.cc | 2 +- winsup/utils/loadlib.h| 6 +-- winsup/utils/mingw/cygcheck.cc| 4 +- winsup/utils/mingw/strace.cc | 4 +- winsup/utils/profiler.cc | 4 +- winsup/utils/ps.cc| 2 +- winsup/utils/regtool.cc | 4 +- 87 files changed, 258 insertions(+), 284 deletions(-) delete mode 100644 winsup/cygwin
Re: [PATCH] Cygwin: Set threadnames with SetThreadDescription()
On 5/29/2022 10:03 AM, Jon Turney wrote: gdb master recently learnt how to use GetThreadDescription() [1], so set threadnames using SetThreadDescription() [available since Windows 101607] as well. This is superior to using a special exception to indicate the thread name to the debugger, because the thread name isn't missed if you don't have a debugger attached at the time it's set. It's not clear what the encoding of a thread name string is, we assume UTF8 for the moment. For the moment, continue to use the old method as well, for the benefit of older gdb versions etc. LGTM, except for a few missing spaces (see below), although maybe you did that deliberately since the existing code was already like that. --- a/winsup/cygwin/miscfuncs.cc +++ b/winsup/cygwin/miscfuncs.cc @@ -18,6 +18,9 @@ details. */ #include "tls_pbuf.h" #include "mmap_alloc.h" +/* not yet prototyped in w32api */ +extern "C" HRESULT WINAPI SetThreadDescription(HANDLE hThread, PCWSTR lpThreadDescription); ^ @@ -993,8 +996,8 @@ wmempcpy: \n\ #define MS_VC_EXCEPTION 0x406D1388 -void -SetThreadName(DWORD dwThreadID, const char* threadName) ^ +static void +SetThreadNameExc(DWORD dwThreadID, const char* threadName) ^ @@ -1025,6 +1028,32 @@ SetThreadName(DWORD dwThreadID, const char* threadName) __endtry } +void +SetThreadName(DWORD dwThreadID, const char* threadName) ^ [...] + SetThreadNameExc(dwThreadID, threadName); ^ Ken
Re: [PATCH 7/7] Cygwin: remove miscellaneous 32-bit code
On 5/29/2022 9:39 AM, Jon Turney wrote: On 26/05/2022 20:17, Ken Brown wrote: winsup/cygwin/autoload.cc | 136 --- Looks good. I think that perhaps the stdcall decoration number n is unused on x86_64, so can be removed also in a followup? Thanks, I missed that. Also, I guess most or all of the uses of __stdcall and __cdecl can be removed from the code. Ken
Re: [PATCH] Cygwin: cygheap: Fix the issue of cygwin1.dll in the root directory.
On 5/28/2022 10:20 AM, Takashi Yano wrote:
- After the commit 6d898f43, cygwin fails to start if cygwin1.dll
is placed in the root directory. This patch fixes the issue.
Addresses: https://cygwin.com/pipermail/cygwin/2022-May/251548.html
---
winsup/cygwin/cygheap.cc | 5 +
1 file changed, 5 insertions(+)
diff --git a/winsup/cygwin/cygheap.cc b/winsup/cygwin/cygheap.cc
index 01b49468e..1a817b743 100644
--- a/winsup/cygwin/cygheap.cc
+++ b/winsup/cygwin/cygheap.cc
@@ -183,6 +183,11 @@ init_cygheap::init_installation_root ()
if (p)
p = wcschr (p + 1, L'\\'); /* Skip share name */
}
+ else /* Long path prefix followed by drive letter path */
+ {
+ len = 4;
+ p += 4;
+ }
}
installation_root_buf[1] = L'?';
RtlInitEmptyUnicodeString (_key, installation_key_buf,
LGTM.
Ken
[PATCH 7/7] Cygwin: remove miscellaneous 32-bit code
Patch attached.From d78b3f13a5eca554d50a7c40532f84cafdd1435a Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 24 May 2022 14:26:33 -0400
Subject: [PATCH 7/7] Cygwin: remove miscellaneous 32-bit code
---
winsup/cygwin/autoload.cc| 136 ---
winsup/cygwin/child_info.h | 4 -
winsup/cygwin/cpuid.h| 23
winsup/cygwin/cygheap.cc | 4 -
winsup/cygwin/cygmalloc.h| 2 -
winsup/cygwin/cygtls.cc | 2 -
winsup/cygwin/cygtls.h | 4 -
winsup/cygwin/dcrt0.cc | 19
winsup/cygwin/dlfcn.cc | 23
winsup/cygwin/dll_init.cc| 11 --
winsup/cygwin/external.cc| 7 --
winsup/cygwin/fhandler_proc.cc | 2 -
winsup/cygwin/fhandler_procnet.cc| 2 -
winsup/cygwin/fhandler_socket_inet.cc| 49
winsup/cygwin/fhandler_socket_local.cc | 10 --
winsup/cygwin/fhandler_socket_unix.cc| 4 -
winsup/cygwin/fork.cc| 4 -
winsup/cygwin/gcc_seh.h | 2 -
winsup/cygwin/heap.cc| 50 +
winsup/cygwin/hookapi.cc | 25 +
winsup/cygwin/include/a.out.h| 7 --
winsup/cygwin/include/asm/bitsperlong.h | 4 -
winsup/cygwin/include/bits/wordsize.h| 6 +-
winsup/cygwin/include/cygwin/config.h| 11 +-
winsup/cygwin/include/cygwin/signal.h| 59 --
winsup/cygwin/include/sys/cygwin.h | 4 -
winsup/cygwin/include/sys/dirent.h | 16 ---
winsup/cygwin/init.cc| 3 -
winsup/cygwin/lib/_cygwin_crt0_common.cc | 11 --
winsup/cygwin/libstdcxx_wrapper.cc | 7 --
winsup/cygwin/miscfuncs.cc | 96
winsup/cygwin/miscfuncs.h| 2 -
winsup/cygwin/mmap.cc| 36 --
winsup/cygwin/mmap_alloc.cc | 4 -
winsup/cygwin/mmap_alloc.h | 4 -
winsup/cygwin/net.cc | 8 --
winsup/cygwin/ntdll.h| 2 -
winsup/cygwin/path.cc| 102 -
winsup/cygwin/perprocess.h | 4 -
winsup/cygwin/shm.cc | 4 -
winsup/cygwin/sigproc.cc | 5 -
winsup/cygwin/smallprint.cc | 32 --
winsup/cygwin/strsig.cc | 14 +--
winsup/cygwin/sysconf.cc | 16 ---
winsup/cygwin/thread.cc | 4 -
winsup/cygwin/wincap.h | 2 -
winsup/cygwin/winsup.h | 10 --
47 files changed, 12 insertions(+), 844 deletions(-)
diff --git a/winsup/cygwin/autoload.cc b/winsup/cygwin/autoload.cc
index 1f52411d8..18366c600 100644
--- a/winsup/cygwin/autoload.cc
+++ b/winsup/cygwin/autoload.cc
@@ -66,7 +66,6 @@ bool NO_COPY wsock_started;
/* LoadDLLprime is used to prime the DLL info information, providing an
additional initialization routine to call prior to calling the first
function. */
-#ifdef __x86_64__
#define LoadDLLprime(dllname, init_also, no_resolve_on_fork) __asm__ ("
\n\
.ifndef " #dllname "_primed\n\
.section .data_cygwin_nocopy,\"w\" \n\
@@ -82,22 +81,6 @@ bool NO_COPY wsock_started;
.set " #dllname "_primed, 1 \n\
.endif \n\
");
-#else
-#define LoadDLLprime(dllname, init_also, no_resolve_on_fork) __asm__ ("
\n\
-.ifndef " #dllname "_primed\n\
- .section .data_cygwin_nocopy,\"w\" \n\
- .align 4 \n\
-."#dllname "_info: \n\
- .long_std_dll_init \n\
- .long" #no_resolve_on_fork " \n\
- .long-1 \n\
- .long" #init_also " \n\
- .string16\"" #dllname ".dll\"\n\
- .text\n\
- .set " #dllname "_primed, 1 \n\
-.endif \n\
-");
-#endif
/* Create a "decorated" name */
#define mangle(name, n) #name "@" #n
@@ -112,7 +95,6 @@ bool NO_COPY wsock_started;
LoadDLLfuncEx3(name, n, dllname, notimp, err, 0)
/* Main DLL setup stuff. */
-#ifdef __x86_64__
#define LoadDLLfuncEx3(name, n, dllname, notimp, err, no_resolve_on_fork) \
LoadDLLprime (dllname, dll_func_load, no_resolve_on_fork) \
__asm__ (" \n\
@@ -137,26 +119,6 @@ _win32_" #name "
[PATCH 6/7] Cygwin: remove 32-bit only clipboard code
Patch attached.From cfa148370cb51c6874d51ee97f79d04f6e547ca9 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 24 May 2022 10:25:06 -0400
Subject: [PATCH 6/7] Cygwin: remove 32-bit only clipboard code
---
winsup/cygwin/fhandler_clipboard.cc | 16
winsup/cygwin/include/sys/clipboard.h | 23 ---
2 files changed, 4 insertions(+), 35 deletions(-)
diff --git a/winsup/cygwin/fhandler_clipboard.cc
b/winsup/cygwin/fhandler_clipboard.cc
index 9515795e4..ae6de4551 100644
--- a/winsup/cygwin/fhandler_clipboard.cc
+++ b/winsup/cygwin/fhandler_clipboard.cc
@@ -68,14 +68,6 @@ fhandler_dev_clipboard::set_clipboard (const void *buf,
size_t len)
clipbuf = (cygcb_t *) GlobalLock (hmem);
clock_gettime (CLOCK_REALTIME, >ts);
-#ifdef __x86_64__
- /* ts overlays cb_sec and cb_nsec such that no conversion is needed */
-#elif __i386__
- /* Expand 32-bit timespec layout to 64-bit layout.
- NOTE: Steps must be done in this order to avoid data loss. */
- clipbuf->cb_nsec = clipbuf->ts.tv_nsec;
- clipbuf->cb_sec = clipbuf->ts.tv_sec;
-#endif
clipbuf->cb_size = len;
memcpy (clipbuf->cb_data, buf, len); // append user-supplied data
@@ -180,14 +172,6 @@ fhandler_dev_clipboard::fstat (struct stat *buf)
&& (hglb = GetClipboardData (format))
&& (clipbuf = (cygcb_t *) GlobalLock (hglb)))
{
-#ifdef __x86_64__
- /* ts overlays cb_sec and cb_nsec such that no conversion is needed */
-#elif __i386__
- /* Compress 64-bit timespec layout to 32-bit layout.
-NOTE: Steps must be done in this order to avoid data loss. */
- clipbuf->ts.tv_sec = clipbuf->cb_sec;
- clipbuf->ts.tv_nsec = clipbuf->cb_nsec;
-#endif
buf->st_atim = buf->st_mtim = clipbuf->ts;
buf->st_size = clipbuf->cb_size;
GlobalUnlock (hglb);
diff --git a/winsup/cygwin/include/sys/clipboard.h
b/winsup/cygwin/include/sys/clipboard.h
index 932fe98d9..e3901d0c8 100644
--- a/winsup/cygwin/include/sys/clipboard.h
+++ b/winsup/cygwin/include/sys/clipboard.h
@@ -17,33 +17,18 @@ details. */
static const WCHAR *CYGWIN_NATIVE = L"CYGWIN_NATIVE_CLIPBOARD";
-/*
- * The following layout of cygcb_t is new with Cygwin 3.3.0. It aids in the
- * transfer of clipboard contents between 32- and 64-bit Cygwin environments.
- */
typedef struct
{
union
{
-/*
- * Note that ts below overlays the struct following it. On 32-bit Cygwin
- * timespec values have to be converted to|from cygcb_t layout. On 64-bit
- * Cygwin timespec values perfectly conform to the struct following, so
- * no conversion is needed.
- *
- * We avoid directly using 'struct timespec' or 'size_t' here because they
- * are different sizes on different architectures. When copy/pasting
- * between 32- and 64-bit Cygwin, the pasted data could appear corrupted,
- * or partially interpreted as a size which can cause an access violation.
- */
-struct timespec ts; // 8 bytes on 32-bit Cygwin, 16 bytes on 64-bit Cygwin
+struct timespec ts;
struct
{
- uint64_t cb_sec; // 8 bytes everywhere
- uint64_t cb_nsec; // 8 bytes everywhere
+ uint64_t cb_sec; // == ts.tv_sec
+ uint64_t cb_nsec; // == ts.tv_nsec
};
};
- uint64_t cb_size; // 8 bytes everywhere
+ uint64_t cb_size;
char cb_data[];
} cygcb_t;
--
2.36.1
[PATCH 5/7] Cygwin: remove some 32-bit only environment code
[PATCH 4/7] Cygwin: remove some 32-bit only path conversion functions
Patch attached.From 1dd3291b22a72edc234b89e55b5bee9ebcc3f158 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 24 May 2022 10:20:23 -0400
Subject: [PATCH 4/7] Cygwin: remove some 32-bit only path conversion functions
---
winsup/cygwin/include/sys/cygwin.h | 23 -
winsup/cygwin/path.cc | 64 --
winsup/testsuite/winsup.api/cygload.cc | 2 -
3 files changed, 89 deletions(-)
diff --git a/winsup/cygwin/include/sys/cygwin.h
b/winsup/cygwin/include/sys/cygwin.h
index 0c3c2d65d..4923caacb 100644
--- a/winsup/cygwin/include/sys/cygwin.h
+++ b/winsup/cygwin/include/sys/cygwin.h
@@ -20,29 +20,6 @@ extern "C" {
#define _CYGWIN_SIGNAL_STRING "cYgSiGw00f"
-#ifdef __i386__
-/* DEPRECATED INTERFACES. These are restricted to MAX_PATH length.
- Don't use in modern applications. They don't exist on x86_64. */
-extern int cygwin_win32_to_posix_path_list (const char *, char *)
- __attribute__ ((__deprecated__));
-extern int cygwin_win32_to_posix_path_list_buf_size (const char *)
- __attribute__ ((__deprecated__));
-extern int cygwin_posix_to_win32_path_list (const char *, char *)
- __attribute__ ((__deprecated__));
-extern int cygwin_posix_to_win32_path_list_buf_size (const char *)
- __attribute__ ((__deprecated__));
-extern int cygwin_conv_to_win32_path (const char *, char *)
- __attribute__ ((__deprecated__));
-extern int cygwin_conv_to_full_win32_path (const char *, char *)
- __attribute__ ((__deprecated__));
-extern int cygwin_conv_to_posix_path (const char *, char *)
- __attribute__ ((__deprecated__));
-extern int cygwin_conv_to_full_posix_path (const char *, char *)
- __attribute__ ((__deprecated__));
-#endif /* __i386__ */
-
-/* Use these interfaces in favor of the above. */
-
/* Possible 'what' values in calls to cygwin_conv_path/cygwin_create_path. */
enum
{
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 98f7aa1db..bd3ffdce6 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -4029,40 +4029,6 @@ cygwin_create_path (cygwin_conv_path_t what, const void
*from)
return to;
}
-#ifdef __i386__
-
-extern "C" int
-cygwin_conv_to_win32_path (const char *path, char *win32_path)
-{
- return cygwin_conv_path (CCP_POSIX_TO_WIN_A | CCP_RELATIVE, path, win32_path,
- MAX_PATH);
-}
-
-extern "C" int
-cygwin_conv_to_full_win32_path (const char *path, char *win32_path)
-{
- return cygwin_conv_path (CCP_POSIX_TO_WIN_A | CCP_ABSOLUTE, path, win32_path,
- MAX_PATH);
-}
-
-/* This is exported to the world as cygwin_foo by cygwin.din. */
-
-extern "C" int
-cygwin_conv_to_posix_path (const char *path, char *posix_path)
-{
- return cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_RELATIVE, path, posix_path,
- MAX_PATH);
-}
-
-extern "C" int
-cygwin_conv_to_full_posix_path (const char *path, char *posix_path)
-{
- return cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_ABSOLUTE, path, posix_path,
- MAX_PATH);
-}
-
-#endif /* __i386__ */
-
/* The realpath function is required by POSIX:2008. */
extern "C" char *
@@ -4211,36 +4177,6 @@ env_PATH_to_posix (const void *win32, void *posix,
size_t size)
size, ENV_CVT));
}
-#ifdef __i386__
-
-extern "C" int
-cygwin_win32_to_posix_path_list_buf_size (const char *path_list)
-{
- return conv_path_list_buf_size (path_list, true);
-}
-
-extern "C" int
-cygwin_posix_to_win32_path_list_buf_size (const char *path_list)
-{
- return conv_path_list_buf_size (path_list, false);
-}
-
-extern "C" int
-cygwin_win32_to_posix_path_list (const char *win32, char *posix)
-{
- return_with_errno (conv_path_list (win32, posix, MAX_PATH,
-CCP_WIN_A_TO_POSIX | CCP_RELATIVE));
-}
-
-extern "C" int
-cygwin_posix_to_win32_path_list (const char *posix, char *win32)
-{
- return_with_errno (conv_path_list (posix, win32, MAX_PATH,
-CCP_POSIX_TO_WIN_A | CCP_RELATIVE));
-}
-
-#endif /* __i386__ */
-
extern "C" ssize_t
cygwin_conv_path_list (cygwin_conv_path_t what, const void *from, void *to,
size_t size)
diff --git a/winsup/testsuite/winsup.api/cygload.cc
b/winsup/testsuite/winsup.api/cygload.cc
index ad4599666..faad5ce0e 100644
--- a/winsup/testsuite/winsup.api/cygload.cc
+++ b/winsup/testsuite/winsup.api/cygload.cc
@@ -152,8 +152,6 @@ cygwin::connector::connector (const char *dll)
// Pick up the function pointers for the basic infrastructure.
get_symbol ("__errno", _errno);
get_symbol ("strerror", _strerror);
- get_symbol ("cygwin_conv_to_full_posix_path", _conv_to_full_posix_path);
- get_symbol ("cygwin_conv_to_full_win32_path", _conv_to_full_win32_path);
// Note that you need to be running an interruptible cygwin function if
// you want to receive signals. You can use the standard signal()
--
2.36.1
[PATCH 2/7] Cygwin: simplify some function names
Patch attached.From 2e8b069182e3ba639beac43794201e409723b5d9 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sun, 22 May 2022 15:18:48 -0400
Subject: [PATCH 2/7] Cygwin: simplify some function names
Remove "32" or "64" from each of the following names: acl32,
aclcheck32, aclfrommode32, aclfrompbits32, aclfromtext32, aclsort32,
acltomode32, acltopbits32, acltotext32, facl32, fchown32, fcntl64,
fstat64, _fstat64, _fstat64_r, ftruncate64, getgid32, getgrent32,
getgrgid32, getgrnam32, getgroups32, getpwuid32, getpwuid_r32,
getuid32, getuid32, initgroups32, lseek64, lstat64, mknod32, mmap64,
setegid32, seteuid32, setgid32, setgroups32, setregid32, setreuid32,
setuid32, stat64, _stat64_r, truncate64.
Remove prototypes and macro definitions of these names.
Remove "#ifndef __INSIDE_CYGWIN__" from some headers so that the new
names will be available when compiling Cygwin.
Remove aliases that are no longer needed.
Include in fhandler_clipboard.cc for the declarations of
geteuid and getegid.
---
newlib/libc/include/pwd.h | 2 +-
newlib/libc/include/sys/stat.h | 2 +-
newlib/libc/include/sys/unistd.h | 16 +---
newlib/libc/posix/posix_spawn.c| 11 ---
winsup/cygwin/cygheap.h| 2 +-
winsup/cygwin/cygmalloc.h | 2 -
winsup/cygwin/cygserver_ipc.h | 4 +-
winsup/cygwin/fcntl.cc | 5 +-
winsup/cygwin/fhandler.cc | 4 +-
winsup/cygwin/fhandler_clipboard.cc| 5 +-
winsup/cygwin/fhandler_disk_file.cc| 2 +-
winsup/cygwin/fhandler_process.cc | 2 +-
winsup/cygwin/fhandler_raw.cc | 4 +-
winsup/cygwin/fhandler_socket_local.cc | 8 +-
winsup/cygwin/fhandler_tty.cc | 2 +-
winsup/cygwin/glob.cc | 7 +-
winsup/cygwin/grp.cc | 28 ++
winsup/cygwin/include/cygwin/acl.h | 2 -
winsup/cygwin/include/cygwin/grp.h | 7 --
winsup/cygwin/include/cygwin/stat.h| 10 --
winsup/cygwin/include/sys/mman.h | 2 -
winsup/cygwin/ipc.cc | 2 +-
winsup/cygwin/libc/fts.c | 9 --
winsup/cygwin/libc/minires.c | 4 +-
winsup/cygwin/libc/rcmd.cc | 13 ++-
winsup/cygwin/libc/rexec.cc| 4 +-
winsup/cygwin/mktemp.cc| 4 +-
winsup/cygwin/mmap.cc | 8 +-
winsup/cygwin/passwd.cc| 8 +-
winsup/cygwin/path.cc | 4 +-
winsup/cygwin/posix_ipc.cc | 10 +-
winsup/cygwin/sec_acl.cc | 41 +++--
winsup/cygwin/security.cc | 4 +-
winsup/cygwin/security.h | 2 -
winsup/cygwin/syscalls.cc | 121 ++---
winsup/cygwin/uinfo.cc | 16 +---
winsup/cygwin/winsup.h | 7 --
37 files changed, 124 insertions(+), 260 deletions(-)
diff --git a/newlib/libc/include/pwd.h b/newlib/libc/include/pwd.h
index baae88410..45d1594ea 100644
--- a/newlib/libc/include/pwd.h
+++ b/newlib/libc/include/pwd.h
@@ -55,10 +55,10 @@ struct passwd {
char*pw_shell; /* default shell */
};
-#ifndef __INSIDE_CYGWIN__
struct passwd *getpwuid (uid_t);
struct passwd *getpwnam (const char *);
+#ifndef __INSIDE_CYGWIN__
#if __MISC_VISIBLE || __POSIX_VISIBLE
int getpwnam_r (const char *, struct passwd *,
char *, size_t , struct passwd **);
diff --git a/newlib/libc/include/sys/stat.h b/newlib/libc/include/sys/stat.h
index 6525272dd..98f5addb4 100644
--- a/newlib/libc/include/sys/stat.h
+++ b/newlib/libc/include/sys/stat.h
@@ -142,7 +142,7 @@ int mkfifo (const char *__path, mode_t __mode );
intstat (const char *__restrict __path, struct stat *__restrict __sbuf );
mode_t umask (mode_t __mask );
-#if defined (__SPU__) || defined(__rtems__) || defined(__CYGWIN__) &&
!defined(__INSIDE_CYGWIN__)
+#if defined (__SPU__) || defined(__rtems__) || defined(__CYGWIN__)
intlstat (const char *__restrict __path, struct stat *__restrict __buf );
intmknod (const char *__path, mode_t __mode, dev_t __dev );
#endif
diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index 3cc313080..216d779d8 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -21,9 +21,7 @@ int access (const char *__path, int __amode);
unsigned alarm (unsigned __secs);
int chdir (const char *__path);
int chmod (const char *__path, mode_t __mode);
-#if !defined(__INSIDE_CYGWIN__)
int chown (const char *__path, uid_t __owner, gid_t __group);
-#endif
#if __BSD_VISIBLE || (__XSI_VISIBLE >= 4 && __POSIX_VISIBLE < 200112)
int chroot (const char *__path);
#endif
@@ -79,11 +77,9 @@ int fchdir (int __fildes);
#if __POSIX_VISIBLE >= 199309
int fchmod (int __fildes, mode_t __mode);
#endif
-#if !defined(__INSIDE_CYGWIN__)
#if
[PATCH 1/7] Cygwin: remove some 32-bit-only function definitions
Patch attached.From 3458c48712e41cef108b7c9582fdc2f63ccac11c Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sun, 22 May 2022 15:16:47 -0400
Subject: [PATCH 1/7] Cygwin: remove some 32-bit-only function definitions
Remove the definitions of the following: acl, aclcheck, aclfrommode,
aclfrompbits, aclfromtext, aclsort, acltomode, acltopbits, acltotext,
chown, fchown, _fcntl, fstat, _fstat_r, ftruncate, getegid, geteuid, getgid,
getgrent, getgrgid, getgrnam, getgroups, getpwduid, getpwuid,
getpwuid_r, getuid, initgroups, lacl, lacl32, lchown, lseek, lstat,
mknod, mmap, setegid, seteuid, setgid, setgroups, setregid, setreuid,
setuid, stat, _stat_r, truncate.
[For most of these, the corresponding 64-bit entry points are obtained
by exporting aliases. For example, acl is an alias for acl32, and
truncate is an alias for truncate64.]
Remove the following structs and all code using them (which is 32-bit
only): __stat32, __group16, __flock32, __aclent16_t.
Remove the typedefs of __blkcnt32_t __dev16_t, __ino32_t, which are
used only in code that has been removed.
Put the typedefs of __uid16_t and __gid16_t in one header, instead of
one header if __INSIDE_CYGWIN__ is defined and a different header
otherwise.
---
winsup/cygwin/fcntl.cc | 41 --
winsup/cygwin/glob.cc | 31
winsup/cygwin/grp.cc | 98 -
winsup/cygwin/include/cygwin/grp.h | 10 --
winsup/cygwin/include/cygwin/stat.h| 19 ---
winsup/cygwin/include/machine/_types.h | 8 -
winsup/cygwin/include/machine/types.h | 11 --
winsup/cygwin/include/sys/cygwin.h | 2 -
winsup/cygwin/mmap.cc | 8 -
winsup/cygwin/passwd.cc| 24 ---
winsup/cygwin/sec_acl.cc | 98 -
winsup/cygwin/security.h | 7 -
winsup/cygwin/syscalls.cc | 195 -
winsup/cygwin/uinfo.cc | 32
14 files changed, 584 deletions(-)
diff --git a/winsup/cygwin/fcntl.cc b/winsup/cygwin/fcntl.cc
index 507ba61f7..437fcf128 100644
--- a/winsup/cygwin/fcntl.cc
+++ b/winsup/cygwin/fcntl.cc
@@ -79,46 +79,5 @@ fcntl64 (int fd, int cmd, ...)
return res;
}
-#ifdef __i386__
-extern "C" int
-_fcntl (int fd, int cmd, ...)
-{
- intptr_t arg = 0;
- va_list args;
- struct __flock32 *src = NULL;
- struct flock dst;
-
- __try
-{
- va_start (args, cmd);
- arg = va_arg (args, intptr_t);
- va_end (args);
- if (cmd == F_GETLK || cmd == F_SETLK || cmd == F_SETLKW)
- {
- src = (struct __flock32 *) arg;
- dst.l_type = src->l_type;
- dst.l_whence = src->l_whence;
- dst.l_start = src->l_start;
- dst.l_len = src->l_len;
- dst.l_pid = src->l_pid;
- arg = (intptr_t)
- }
- int res = fcntl64 (fd, cmd, arg);
- if (cmd == F_GETLK)
- {
- src->l_type = dst.l_type;
- src->l_whence = dst.l_whence;
- src->l_start = dst.l_start;
- src->l_len = dst.l_len;
- src->l_pid = (short) dst.l_pid;
- }
- return res;
-}
- __except (EFAULT)
- __endtry
- return -1;
-}
-#else
EXPORT_ALIAS (fcntl64, fcntl)
EXPORT_ALIAS (fcntl64, _fcntl)
-#endif
diff --git a/winsup/cygwin/glob.cc b/winsup/cygwin/glob.cc
index b0d393f98..b14cf6dab 100644
--- a/winsup/cygwin/glob.cc
+++ b/winsup/cygwin/glob.cc
@@ -855,38 +855,7 @@ g_opendir(Char *str, glob_t *pglob)
return(opendir(buf));
}
-#ifdef __x86_64__
#define CYGWIN_gl_stat(sfptr) ((*pglob->sfptr) (buf, sb))
-#else
-static void
-stat32_to_stat64 (struct __stat32 *src, struct stat *dst)
-{
- dst->st_dev = src->st_dev;
- dst->st_ino = src->st_ino;
- dst->st_mode = src->st_mode;
- dst->st_nlink = src->st_nlink;
- dst->st_uid = src->st_uid;
- dst->st_gid = src->st_gid;
- dst->st_rdev = src->st_rdev;
- dst->st_size = src->st_size;
- dst->st_atim = src->st_atim;
- dst->st_mtim = src->st_mtim;
- dst->st_ctim = src->st_ctim;
- dst->st_birthtim = src->st_mtim;
- dst->st_blksize = src->st_blksize;
- dst->st_blocks = src->st_blocks;
-}
-
-#define CYGWIN_gl_stat(sfptr) \
- ({ int ret; \
- struct __stat32 lsb; \
- if (CYGWIN_VERSION_CHECK_FOR_USING_BIG_TYPES) \
- ret = (*pglob->sfptr) (buf, sb);
\
- else if (!(ret = (*pglob->sfptr) (buf, (struct stat *) ))) \
- stat32_to_stat64 (, sb); \
- ret; \
- })
-#endif
static int
g_lstat(Char *fn, struct stat *sb, glob_t *pglob)
diff --git a/winsup/cygwin/grp.cc b/winsup/cygwin/grp.cc
index 23e2d0251..893e4cc0f 100644
--- a/winsup/cygw
[PATCH 0/7] Remove 32-bit code
The patches in this series other than the second remove code that was used only in the 32-bit build. The second patch does some code simpification that is possible as a result of the removal. For example, we rename stat64 to stat. This eliminates the need to export stat as an alias for stat64, and it also eliminates the need to define a prototype of stat64 for use in the build of Cygwin. Ken Brown (7): Cygwin: remove some 32-bit-only function definitions Cygwin: simplify some function names Cygwin: remove regparm.h Cygwin: remove some 32-bit only path conversion functions Cygwin: remove some 32-bit only environment code Cygwin: remove 32-bit only clipboard code Cygwin: remove miscellaneous 32-bit code newlib/libc/include/pwd.h| 2 +- newlib/libc/include/sys/stat.h | 2 +- newlib/libc/include/sys/unistd.h | 16 +- newlib/libc/posix/posix_spawn.c | 11 - winsup/cygwin/autoload.cc| 136 -- winsup/cygwin/child_info.h | 14 +- winsup/cygwin/cpuid.h| 23 -- winsup/cygwin/cygerrno.h | 11 +- winsup/cygwin/cygheap.cc | 42 ++- winsup/cygwin/cygheap.h | 18 +- winsup/cygwin/cygheap_malloc.h | 24 +- winsup/cygwin/cygmalloc.h| 25 +- winsup/cygwin/cygserver_ipc.h| 4 +- winsup/cygwin/cygthread.h| 4 +- winsup/cygwin/cygtls.cc | 2 - winsup/cygwin/cygtls.h | 26 +- winsup/cygwin/cygwait.h | 2 +- winsup/cygwin/dcrt0.cc | 41 +-- winsup/cygwin/debug.cc | 6 +- winsup/cygwin/debug.h| 10 +- winsup/cygwin/dlfcn.cc | 23 -- winsup/cygwin/dll_init.cc| 36 --- winsup/cygwin/dll_init.h | 6 - winsup/cygwin/dtable.h | 4 +- winsup/cygwin/environ.cc | 31 +-- winsup/cygwin/environ.h | 19 +- winsup/cygwin/errno.cc | 8 +- winsup/cygwin/exceptions.cc | 4 +- winsup/cygwin/external.cc| 7 - winsup/cygwin/fcntl.cc | 46 +--- winsup/cygwin/fhandler.cc| 18 +- winsup/cygwin/fhandler.h | 324 +++ winsup/cygwin/fhandler_clipboard.cc | 25 +- winsup/cygwin/fhandler_console.cc| 14 +- winsup/cygwin/fhandler_cygdrive.cc | 2 +- winsup/cygwin/fhandler_dev.cc| 4 +- winsup/cygwin/fhandler_dev_fd.cc | 2 +- winsup/cygwin/fhandler_disk_file.cc | 30 +-- winsup/cygwin/fhandler_dsp.cc| 8 +- winsup/cygwin/fhandler_fifo.cc | 6 +- winsup/cygwin/fhandler_floppy.cc | 4 +- winsup/cygwin/fhandler_mqueue.cc | 4 +- winsup/cygwin/fhandler_netdrive.cc | 2 +- winsup/cygwin/fhandler_pipe.cc | 8 +- winsup/cygwin/fhandler_proc.cc | 4 +- winsup/cygwin/fhandler_process.cc| 4 +- winsup/cygwin/fhandler_process_fd.cc | 2 +- winsup/cygwin/fhandler_procnet.cc| 4 +- winsup/cygwin/fhandler_procsys.cc| 4 +- winsup/cygwin/fhandler_procsysvipc.cc| 2 +- winsup/cygwin/fhandler_random.cc | 2 +- winsup/cygwin/fhandler_raw.cc| 6 +- winsup/cygwin/fhandler_registry.cc | 2 +- winsup/cygwin/fhandler_serial.cc | 4 +- winsup/cygwin/fhandler_signalfd.cc | 4 +- winsup/cygwin/fhandler_socket.cc | 4 +- winsup/cygwin/fhandler_socket_inet.cc| 51 +--- winsup/cygwin/fhandler_socket_local.cc | 22 +- winsup/cygwin/fhandler_socket_unix.cc| 10 +- winsup/cygwin/fhandler_tape.cc | 6 +- winsup/cygwin/fhandler_timerfd.cc| 4 +- winsup/cygwin/fhandler_tty.cc| 14 +- winsup/cygwin/fhandler_virtual.cc| 4 +- winsup/cygwin/fhandler_windows.cc| 2 +- winsup/cygwin/fhandler_zero.cc | 2 +- winsup/cygwin/fork.cc| 4 - winsup/cygwin/gcc_seh.h | 2 - winsup/cygwin/glob.cc| 38 +-- winsup/cygwin/globals.cc | 6 - winsup/cygwin/grp.cc | 126 + winsup/cygwin/heap.cc| 52 +--- winsup/cygwin/hookapi.cc | 25 +- winsup/cygwin/include/a.out.h| 7 - winsup/cygwin/include/asm/bitsperlong.h | 4 - winsup/cygwin/include/bits/wordsize.h| 6 +- winsup/cygwin/include/cygwin/acl.h | 2 - winsup/cygwin/include/cygwin/config.h| 11 +- winsup/cygwin/include/cygwin/grp.h | 17 -- winsup/cygwin/include/cygwin/signal.h| 59 - winsup/cygwin/include/cygwin/stat.h | 29 -- winsup/cygwin/include/machine/_types.h | 8 - winsup/cygwin/include/machine/types.h| 11 - winsup/cygwin/include/sys/clipboard.h
Re: [PATCH] fhandler_proc.cc(format_proc_cpuinfo): add Linux 5.18 cpuinfo flags
On 5/25/2022 8:30 AM, Brian Inglis wrote: 0x8008:0 EBX:31 brs AMD Branch Sampling available 0x8022:0 EAX:0 perfmon_v2 AMD ExtPerfMonAndDbg Performance Monitoring Version 2 0x0021:0 EBX|EDX|ECX=="IntelTDX" tdx_guest Intel Trust Domain Extensions- Guest Support --- winsup/cygwin/fhandler_proc.cc | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) Pushed. Thanks. Ken
[PATCH] Cygwin: fix mknod (64-bit only)
Patch attached. I'll push it in a few days if no one sees anything wrong with
it.
KenFrom 54cfc70ff3a548d861cb59ac6b2084cc6b791ec2 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sun, 22 May 2022 11:43:44 -0400
Subject: [PATCH] Cygwin: fix mknod (64-bit only)
The current definition of mknod in syscalls.cc has a third argument of
type __dev16_t instead of dev_t. Fix this on 64-bit Cygwin by
exporting mknod as an alias for mknod32. (There is no problem on
32-bit because mknod is redirected to mknod32 via NEW_FUNCTIONS in
Makefile.am.)
Addresses: https://cygwin.com/pipermail/cygwin-developers/2022-May/012589.html
---
winsup/cygwin/release/3.3.6 | 3 +++
winsup/cygwin/syscalls.cc | 4
2 files changed, 7 insertions(+)
diff --git a/winsup/cygwin/release/3.3.6 b/winsup/cygwin/release/3.3.6
index 6d722433f..135f33155 100644
--- a/winsup/cygwin/release/3.3.6
+++ b/winsup/cygwin/release/3.3.6
@@ -7,3 +7,6 @@ Bug Fixes
- Fix killpg failing because the exec'ing as well as the exec'ed
process are not in the pidlist for a brief moment.
Addresses: https://cygwin.com/pipermail/cygwin/2022-May/251479.html
+
+- Fix mknod (64-bit only), whose definition didn't match its prototype.
+ Addresses:
https://cygwin.com/pipermail/cygwin-developers/2022-May/012589.html
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 3a652c4f4..344d1d329 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -3490,11 +3490,15 @@ mknod32 (const char *path, mode_t mode, dev_t dev)
return -1;
}
+#ifdef __i386__
extern "C" int
mknod (const char *_path, mode_t mode, __dev16_t dev)
{
return mknod32 (_path, mode, (dev_t) dev);
}
+#else
+EXPORT_ALIAS (mknod32, mknod)
+#endif
extern "C" int
mkfifo (const char *path, mode_t mode)
--
2.36.1
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/26/2021 6:12 PM, Ken Brown wrote: There are some fixes (though not pipe-related) pending for 3.3.4, so I'll push it to the 3.3 branch after I've heard from Takashi and/or Corinna. Takashi must be unavailable also, but it's a simple enough fix that I decided to go ahead and push it. Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/26/2021 6:23 PM, Jeremy Drake wrote: On Sun, 26 Dec 2021, Ken Brown wrote: On 12/26/2021 5:43 PM, Jeremy Drake wrote: My loops are still going after an hour. I know that ARM64 would have hit the assert before now. Well, ARM64 hung up, but didn't hit the assert, so maybe there's some *other* issue running around. Unfortunately gdb doesn't work to attach to a process there (it gets confused with the ARM64 DLLs loaded in the process). And WinDbg can't load the symbols for a cygwin dll (cv2pdb generally works pretty well for this, but it seems to be confused by the split .dbg file for msys-2.0.dll). Sounds like nothing can be done until it shows up on x86_64. Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/26/2021 5:43 PM, Jeremy Drake wrote: On Sun, 26 Dec 2021, Ken Brown wrote: + /* NtQueryInformationProcess can return STATUS_SUCCESS with +invalid handle data for certain processes. See + https://github.com/processhacker/processhacker/blob/master/phlib/native.c#L5754. I would recommend using the "permalink" to the line, since future commits could change both the line number and even the comment you are referring to. https://github.com/processhacker/processhacker/blob/05f5e9fa477dcaa1709d9518170d18e1b3b8330d/phlib/native.c#L5754 Good idea, thanks. +We need to ensure that NumberOfHandles is zero in this +case to avoid a crash in the loop below. */ + phi->NumberOfHandles = 0; status = NtQueryInformationProcess (proc, ProcessHandleInformation, phi, nbytes, ); if (NT_SUCCESS (status)) Would it make sense to leave an assert (phi->NumberOfHandles <= n_handle) before the for loop too just in case something odd happens in the future? That made it a lot easier to know what was going on. Yes, I think so. My loops are still going after an hour. I know that ARM64 would have hit the assert before now. Would this also be pushed to the 3.3 branch? Or are there plans to make a 3.3.4 at some point? I saw a pipe-related hang reported to MSYS2 (that I didn't see this issue in the stack traces), but I am not sure if there are any more pipe fixes pending post 3.3.3. There are some fixes (though not pipe-related) pending for 3.3.4, so I'll push it to the 3.3 branch after I've heard from Takashi and/or Corinna. Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/26/2021 4:35 PM, Jeremy Drake wrote:
On Sun, 26 Dec 2021, Ken Brown wrote:
On 12/26/2021 11:04 AM, Ken Brown wrote:
On 12/26/2021 10:09 AM, Ken Brown wrote:
1. For some processes, NtQueryInformationProcess(ProcessHandleInformation)
can return STATUS_SUCCESS with invalid handle information. See the
comment starting at line 5754, where it is shown how to detect this.
I kind of thought something like this (that NumberOfHandles was
uninitialized memory).
If I'm right, the following patch should fix the problem:
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index ba6b70f55..4cef3e4ca 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -1228,6 +1228,7 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
HeapAlloc (GetProcessHeap (), 0, nbytes);
if (!phi)
goto close_proc;
+ phi->NumberOfHandles = 0;
status = NtQueryInformationProcess (proc,
ProcessHandleInformation,
phi, nbytes, );
if (NT_SUCCESS (status))
Actually, this first hunk should suffice.
Jeremy, could you try this?
Ken
I've built (leaving the assert in place too), and I've got 3 loops going
on server 2022 and 1 going on ARM64. So far so good. I don't know how
long before calling it good though.
Great, thanks for testing. I'm attaching the complete patch (with
documentation). I'll push it once you're convinced that it fixes the problem,
assuming Takashi agrees. (I think Corinna is unavailable.)
KenFrom 4858e73321a0618a8b1e1060416ef7d546cda895 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sun, 26 Dec 2021 16:42:26 -0500
Subject: [PATCH] Cygwin: fhandler_pipe::get_query_hdl_per_process: avoid a
crash
NtQueryInformationProcess(ProcessHandleInformation) can return
STATUS_SUCCESS with invalid handle data for certain processes
("minimal" processes on Windows 10). This can cause a crash when
there's an attempt to access that data. Fix that by setting
NumberOfHandles to zero before calling NtQueryInformationProcess.
Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q4/011611.html
---
winsup/cygwin/fhandler_pipe.cc | 6 ++
winsup/cygwin/release/3.3.4| 3 +++
2 files changed, 9 insertions(+)
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index 25a092262..2674d154c 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -1256,6 +1256,12 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
HeapAlloc (GetProcessHeap (), 0, nbytes);
if (!phi)
goto close_proc;
+ /* NtQueryInformationProcess can return STATUS_SUCCESS with
+invalid handle data for certain processes. See
+
https://github.com/processhacker/processhacker/blob/master/phlib/native.c#L5754.
+We need to ensure that NumberOfHandles is zero in this
+case to avoid a crash in the loop below. */
+ phi->NumberOfHandles = 0;
status = NtQueryInformationProcess (proc, ProcessHandleInformation,
phi, nbytes, );
if (NT_SUCCESS (status))
diff --git a/winsup/cygwin/release/3.3.4 b/winsup/cygwin/release/3.3.4
index a15684fdb..048426942 100644
--- a/winsup/cygwin/release/3.3.4
+++ b/winsup/cygwin/release/3.3.4
@@ -14,3 +14,6 @@ Bug Fixes
rather than io_handle while neither read() nor select() is called
after the cygwin app is started from non-cygwin app.
Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q4/011587.html
+
+- Avoid a crash when NtQueryInformationProcess returns invalid handle data.
+ Addresses: https://cygwin.com/pipermail/cygwin-patches/2021q4/011611.html
--
2.34.1
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/26/2021 11:04 AM, Ken Brown wrote:
On 12/26/2021 10:09 AM, Ken Brown wrote:
1. For some processes, NtQueryInformationProcess(ProcessHandleInformation) can
return STATUS_SUCCESS with invalid handle information. See the comment
starting at line 5754, where it is shown how to detect this.
If I'm right, the following patch should fix the problem:
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index ba6b70f55..4cef3e4ca 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -1228,6 +1228,7 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
HeapAlloc (GetProcessHeap (), 0, nbytes);
if (!phi)
goto close_proc;
+ phi->NumberOfHandles = 0;
status = NtQueryInformationProcess (proc, ProcessHandleInformation,
phi, nbytes, );
if (NT_SUCCESS (status))
Actually, this first hunk should suffice.
@@ -1238,6 +1239,11 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
while (n_handle < (1L<<20) && status == STATUS_INFO_LENGTH_MISMATCH);
if (!NT_SUCCESS (status))
goto close_proc;
+ if (phi->NumberOfHandles == 0)
+ {
+ HeapFree (GetProcessHeap (), 0, phi);
+ goto close_proc;
+ }
for (ULONG j = 0; j < phi->NumberOfHandles; j++)
{
Jeremy, could you try this?
Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/26/2021 10:09 AM, Ken Brown wrote:
1. For some processes, NtQueryInformationProcess(ProcessHandleInformation) can
return STATUS_SUCCESS with invalid handle information. See the comment starting
at line 5754, where it is shown how to detect this.
If I'm right, the following patch should fix the problem:
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index ba6b70f55..4cef3e4ca 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -1228,6 +1228,7 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
HeapAlloc (GetProcessHeap (), 0, nbytes);
if (!phi)
goto close_proc;
+ phi->NumberOfHandles = 0;
status = NtQueryInformationProcess (proc, ProcessHandleInformation,
phi, nbytes, );
if (NT_SUCCESS (status))
@@ -1238,6 +1239,11 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
while (n_handle < (1L<<20) && status == STATUS_INFO_LENGTH_MISMATCH);
if (!NT_SUCCESS (status))
goto close_proc;
+ if (phi->NumberOfHandles == 0)
+ {
+ HeapFree (GetProcessHeap (), 0, phi);
+ goto close_proc;
+ }
for (ULONG j = 0; j < phi->NumberOfHandles; j++)
{
Jeremy, could you try this?
Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/25/2021 11:56 PM, Jeremy Drake wrote: I set up a windows server 2022 VM last night and went nuts stressing pacman/GPGME. I was able to reproduce the issue there: status = 0x, phi->NumberOfHandles = 8261392, n_handle = 256 [#--] 14% assertion "phi->NumberOfHandles <= n_handle" failed: file "../../.././winsup/cygwin/fhandler_pipe.cc", line 1281, function: void* fhandler_pipe::get_query_hdl_per_process(WCHAR*, OBJECT_NAME_INFORMATION*) So it is not something inherent in the x86_64-on-ARM64 emulation but can happen on native x86_64 also. A Google search led me to something that might explain what's going on. Look at the function PhEnumHandlesEx2 starting at line 5713 in https://github.com/processhacker/processhacker/blob/master/phlib/native.c#L5152 Two interesting things: 1. For some processes, NtQueryInformationProcess(ProcessHandleInformation) can return STATUS_SUCCESS with invalid handle information. See the comment starting at line 5754, where it is shown how to detect this. 2. You can use the ReturnLength parameter of NtQueryInformationProcess to see how big a buffer is needed. This might be more efficient than repeatedly doubling the buffer size. Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/25/2021 6:00 PM, Jeremy Drake via Cygwin-patches wrote: On Sat, 25 Dec 2021, Jeremy Drake via Cygwin-patches wrote: On Sun, 26 Dec 2021, Takashi Yano wrote: Could you please check the result of the following test case in that ARM64 platform? OK, on Windows 11 ARM64 (same machine as I was testing the assert on): per_process: n_handle=52, NumberOfHandles=52 per_system: n_handle=65536, NumberOfHandles=35331 On GitHub "windows-2022" runner: per_process: n_handle=63, NumberOfHandles=63 per_system: n_handle=65536, NumberOfHandles=34077 On Windows 10 x86_64 (can't remember if it was 21H1 or 21H2): per_process: n_handle=37, NumberOfHandles=37 per_system: n_handle=131072, NumberOfHandles=76069 This completely shoots down the speculation in my last email. Nevertheless, the results you posted earlier do indicate that *sometimes* NtQueryInformationProcess(ProcessHandleInformation) returns STATUS_SUCCESS even if the buffer it's passed is too small. I hope Takashi has an idea what's going on. Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/25/2021 2:20 PM, Jeremy Drake via Cygwin-patches wrote: On Sun, 26 Dec 2021, Takashi Yano wrote: Could you please check the result of the following test case in that ARM64 platform? I will probably not be able to get to this until tomorrow at the earliest. But keep in mind the issue I'm seeing is not deterministic - I have to run pacman in a loop validating files via GPGME and eventually it will hang (or hit the assert I added in that version). Most of the time, it's perfectly fine. The results you've already posted seem to indicate that, on your platform, NtQueryInformationProcess(ProcessHandleInformation) returns STATUS_SUCCESS even if the buffer it's passed is too small. [That won't necessarily cause a problem in every one of your pacman runs, so it might appear non-deterministic.] Takashi's test case is designed to verify that that's what's going on. And I think he also wants to see if phi->NumberOfHandles is reliable on your platform even when the buffer is too small. If so, then (on your platform), the do-while loop could be replaced by two calls to NtQueryInformationProcess. The first call would determine how big a buffer is needed, and the second call would use a buffer of that size. But we don't know any of that for sure yet. We also don't know (or at least I don't know) what aspects of your platform are relevant. For example, does this always happen on Windows 11? or on ARM64? Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/24/2021 2:42 PM, Jeremy Drake wrote: On Fri, 24 Dec 2021, Ken Brown wrote: I agree that it's hard to see how the line you quoted could cause an exception. But you were using an optimized build, so I'm not sure how reliable the line-number information is. Is it feasible to run your test under strace? If so, you could add some debug_printf statements to examine the values of n_handle and phi->NumberOfHandles. Or what about simply adding an assertion that phi->NumberOfHandles <= n_handle? Ken This issue is not consistent, I was able to reproduce once on x64 by running commands in a loop overnight, but the next time I tried to reproduce I ran for over 24 hours without hitting it. It does seem to happen much more often on Windows on ARM64 (so much so that at first I thought it was an issue with their emulation). With this patch I have not seen the issue again. So can you test your diagnosis by removing your patch and adding an assertion? Also, it seems to have started cropping up in msys2's CI when the GHA runner was switched from "windows-2019" to "windows-2022". And does your patch help here too? I forgot to give a full link to the MSYS2 issue where I have been investigating this: https://github.com/msys2/MSYS2-packages/issues/2752 Actually I think I might see a small bug in the code. But even if I'm right, it would result in n_handle being unnecessarily big rather than too small, so it wouldn't explain what you're seeing. Takashi, in fhandler_pipe.cc:1225, shouldn't you use offsetof(struct _PROCESS_HANDLE_SNAPSHOT_INFORMATION, Handles) instead of 2*sizeof(ULONG_PTR), to take account of possible padding? (And there's a similar issue in fhandler_pipe.cc:1296.) Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/23/2021 7:29 PM, Jeremy Drake via Cygwin-patches wrote: On Thu, 23 Dec 2021, Ken Brown wrote: - for (ULONG j = 0; j < phi->NumberOfHandles; j++) + for (ULONG j = 0; j < min(phi->NumberOfHandles, n_handle); j++) Reading the preceding code, I don't see how n_handle could be less than phi->NumberOfHandles. Can you explain? Not really. I saw this stack trace: ... #3 0x000180062f13 in exception::handle (e=0x14cc4f0, frame=, in=, dispatch=) at /c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/exceptions.cc:835 #4 0x7ff8abd320cf in ntdll!.chkstk () from /c/Windows/SYSTEM32/ntdll.dll #5 0x7ff8abce1454 in ntdll!RtlRaiseException () from /c/Windows/SYSTEM32/ntdll.dll #6 0x7ff8abd30bfe in ntdll!KiUserExceptionDispatcher () from /c/Windows/SYSTEM32/ntdll.dll #7 0x000180092687 in fhandler_pipe::get_query_hdl_per_process (this=this@entry=0x1803700f8, name=name@entry=0x14cc820 L"\\Device\\NamedPipe\\dd50a72ab4668b33-10348-pipe-nt-0x6E6", ntfn=ntfn@entry=0x8000c2ce0) at /c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler_pipe.cc:1281 #8 0x000180092bdb in fhandler_pipe::temporary_query_hdl (this=this@entry=0x1803700f8) at /c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler_pipe.cc:1190 ... Line 1281 of fhandler_pipe.cc was if (phi->Handles[j].GrantedAccess != access) The only way I could see that causing an exception was if it was reading past the end of the allocated memory, if j was greater than (or equal to) n_handle. Unfortunately, I haven't been able to catch it in a debugger again, so I can't confirm this. I took a core with 'dumper' but gdb doesn't want to load it (it says Core file format not supported, maybe something with msys2's gdb?). I agree that it's hard to see how the line you quoted could cause an exception. But you were using an optimized build, so I'm not sure how reliable the line-number information is. Is it feasible to run your test under strace? If so, you could add some debug_printf statements to examine the values of n_handle and phi->NumberOfHandles. Or what about simply adding an assertion that phi->NumberOfHandles <= n_handle? Ken
Re: [PATCH] fhandler_pipe: add sanity limit to handle loops
On 12/23/2021 6:10 PM, Jeremy Drake via Cygwin-patches wrote:
diff --git a/winsup/cygwin/fhandler_pipe.cc
b/winsup/cygwin/fhandler_pipe.cc
index ba6b70f55..48713a38d 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -1239,7 +1239,7 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
if (!NT_SUCCESS (status))
goto close_proc;
- for (ULONG j = 0; j < phi->NumberOfHandles; j++)
+ for (ULONG j = 0; j < min(phi->NumberOfHandles, n_handle); j++)
Reading the preceding code, I don't see how n_handle could be less than
phi->NumberOfHandles. Can you explain?
{
/* Check for the peculiarity of cygwin read pipe */
const ULONG access = FILE_READ_DATA | FILE_READ_EA
@@ -1309,7 +1309,7 @@ fhandler_pipe::get_query_hdl_per_system (WCHAR *name,
if (!NT_SUCCESS (status))
return NULL;
- for (LONG i = (LONG) shi->NumberOfHandles - 1; i >= 0; i--)
+ for (LONG i = (LONG) min(shi->NumberOfHandles, n_handle) - 1; i >= 0; i--)
Same comment.
Ken
[PATCH] Cygwin: fhandler_fifo::raw_read: handle STATUS_PENDING
Patch attached. Takashi, since you wrote the analogous patch for pipes, could
you take a look?
Thanks.
KenFrom 4f47e64b11ed8d47c62fa89e9b971f44b7e9ab75 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 23 Nov 2021 11:40:56 -0500
Subject: [PATCH] Cygwin: fhandler_fifo::raw_read: handle STATUS_PENDING
NtReadFile can return STATUS_PENDING occasionally even in non-blocking
mode. Check for this and wait for NtReadFile to complete. To avoid
code repetition, do this in a static helper function nt_read.
---
winsup/cygwin/fhandler_fifo.cc | 70 +++---
1 file changed, 47 insertions(+), 23 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 489ba528c..34bd835ae 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -1201,12 +1201,39 @@ fhandler_fifo::release_select_sem (const char *from)
ReleaseSemaphore (select_sem, n_release, NULL);
}
+/* Read from a non-blocking pipe and wait for completion. */
+static NTSTATUS
+nt_read (HANDLE h, HANDLE evt, PIO_STATUS_BLOCK pio, void *in_ptr, size_t& len)
+{
+ NTSTATUS status;
+
+ ResetEvent (evt);
+ status = NtReadFile (h, evt, NULL, NULL, pio, in_ptr, len, NULL, NULL);
+ if (status == STATUS_PENDING)
+{
+ /* Very short-lived */
+ status = NtWaitForSingleObject (evt, FALSE, NULL);
+ if (NT_SUCCESS (status))
+ status = pio->Status;
+}
+ return status;
+}
+
void __reg3
fhandler_fifo::raw_read (void *in_ptr, size_t& len)
{
+ HANDLE evt;
+
if (!len)
return;
+ if (!(evt = CreateEvent (NULL, false, false, NULL)))
+{
+ __seterrno ();
+ len = (size_t) -1;
+ return;
+}
+
while (1)
{
int nconnected = 0;
@@ -1244,17 +1271,15 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
NTSTATUS status;
IO_STATUS_BLOCK io;
- status = NtReadFile (fc_handler[j].h, NULL, NULL, NULL,
- , in_ptr, len, NULL, NULL);
+ status = nt_read (fc_handler[j].h, evt, , in_ptr, len);
switch (status)
{
case STATUS_SUCCESS:
case STATUS_BUFFER_OVERFLOW:
- /* io.Information is supposedly valid in latter case. */
if (io.Information > 0)
{
len = io.Information;
- goto out;
+ goto unlock_out;
}
break;
case STATUS_PIPE_EMPTY:
@@ -1265,7 +1290,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
fc_handler[j].set_state (fc_disconnected);
break;
default:
- debug_printf ("NtReadFile status %y", status);
+ debug_printf ("nt_read status %y", status);
fc_handler[j].set_state (fc_error);
break;
}
@@ -1278,8 +1303,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
NTSTATUS status;
IO_STATUS_BLOCK io;
- status = NtReadFile (fc_handler[i].h, NULL, NULL, NULL,
-, in_ptr, len, NULL, NULL);
+ status = nt_read (fc_handler[i].h, evt, , in_ptr, len);
switch (status)
{
case STATUS_SUCCESS:
@@ -1290,7 +1314,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
if (j < nhandlers)
fc_handler[j].last_read = false;
fc_handler[i].last_read = true;
- goto out;
+ goto unlock_out;
}
break;
case STATUS_PIPE_EMPTY:
@@ -1301,7 +1325,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
fc_handler[i].set_state (fc_disconnected);
break;
default:
- debug_printf ("NtReadFile status %y", status);
+ debug_printf ("nt_read status %y", status);
fc_handler[i].set_state (fc_error);
break;
}
@@ -1315,8 +1339,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
IO_STATUS_BLOCK io;
nconnected++;
- status = NtReadFile (fc_handler[i].h, NULL, NULL, NULL,
-, in_ptr, len, NULL, NULL);
+ status = nt_read (fc_handler[i].h, evt, , in_ptr, len);
switch (status)
{
case STATUS_SUCCESS:
@@ -1327,7 +1350,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
if (j < nhandlers)
fc_handler[j].last_read = false;
fc_handler[i].last_read = true;
- goto out;
+ goto unlock_out;
}
break;
case STATUS_PIPE_EMPTY:
@@ -1337,25 +1360,25 @@ fhandler_fifo::raw_read (void *in_ptr,
[PATCH] Cygwin: fhandler_pipe::raw_read: fix handle leak
Patch attached.From 6d34b62cb8e192071e193516c23419854c3b4127 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 23 Nov 2021 10:13:43 -0500
Subject: [PATCH] Cygwin: fhandler_pipe::raw_read: fix handle leak
Slightly rearrange the code to avoid returning without closing the
event handle.
---
winsup/cygwin/fhandler_pipe.cc | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index 3cbd434b7..5195b2807 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -284,13 +284,6 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
if (!len)
return;
- if (!(evt = CreateEvent (NULL, false, false, NULL)))
-{
- __seterrno ();
- len = (size_t) -1;
- return;
-}
-
DWORD timeout = is_nonblocking () ? 0 : INFINITE;
DWORD waitret = cygwait (read_mtx, timeout);
switch (waitret)
@@ -314,6 +307,15 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
len = (size_t) -1;
return;
}
+
+ if (!(evt = CreateEvent (NULL, false, false, NULL)))
+{
+ __seterrno ();
+ len = (size_t) -1;
+ ReleaseMutex (read_mtx);
+ return;
+}
+
while (nbytes < len)
{
ULONG_PTR nbytes_now = 0;
--
2.33.0
Re: [PATCH] Cygwin: pipe: Suppress unnecessary set_pipe_non_blocking() call.
On 11/17/2021 3:08 AM, Takashi Yano wrote: - Call set_pipe_non_blocking(false) only if the pipe will be really inherited to non-cygwin process. LGTM, but Corinna should probably take a quick look too, since I'm not very familiar with this part of the code. Ken
Re: [PATCH v3] Cygwin: pipe: Handle STATUS_PENDING even for nonblocking mode.
On 11/16/2021 9:33 AM, Takashi Yano wrote: - NtReadFile() and NtWriteFile() seems to return STATUS_PENDING occasionally even in nonblocking mode. This patch adds handling for STATUS_PENDING in nonblocking mode. I haven't tested (I assume you have), but LGTM except for two typos below. +- Fix issue that pipe read()/write() occationally returns a garnage occasionally garbage Ken
Re: [PATCH 0/2] Fix a bad case of absolute path handling
On 11/11/2021 4:47 AM, Corinna Vinschen wrote: On Nov 10 17:22, Ken Brown wrote: I can't immediately think of anything. But is it really impossible to phase out DOS path support over a period of time? We could start with a HEADS-UP, asking for comments, then a deprecation announcement, then something like the old dosfilewarning option, then a more forceful warning that can't be turned off, and finally removal of support. This could be done over a period of several years (not sure how many). Yeah, we might try again. Just not over years, we'll probably lose track over time. I'd appreciate a shorter period with a chance to see the end. The problem is that people are probably using DOS paths all the time. Makefiles and scripts mixing Cygwin and DOS tools come to mind. So maybe an RFC email would be useful rather than a HEADS-UP, just to see how widespread it is. For the time being, I wonder if we could start with isabspath being always strict so at least X: isn't an abspath at all. Sounds reasonable. We could also put lines like # C:/ on /c type ntfs (binary,posix=0) into the default /etc/fstab. Commented out, you mean? Just as hint? Yes. We could do that. Personally I don't like these shortcuts, I rather use a shorter cygdrive prefix, like /mnt, but I see how this could convince people. Scripts with mixed tools will always be a problem, though. Ken
Re: [PATCH 0/2] Fix a bad case of absolute path handling
On 11/10/2021 3:32 PM, corinna-cyg...@cygwin.com wrote: From: Corinna Vinschen As I told Takashi in PM, I will try to more often send patches to the cygwin-patches ML before pushing them, so there's a chance to chime in. LGTM. This patch series is supposed to address the `rm -rf' problem reported in https://cygwin.com/pipermail/cygwin/2021-November/249837.html It was always frustrating, having to allow DOS drive letter paths for backward compatibility. This here is another case of ambiguity, triggered by the `isabspath' macro handling "X:" as absolute path, even without the trailing slash or backslash. Check out the 2nd patch for a more detailed description. While at it, I wonder if we might have a chance to fix these ambiguities in a better way. For instance, consider this: $ mkdir -p test/c: $ cd test As non-admin: $ touch c:/foo touch: cannot touch 'c:/foo': Permission denied As admin, even worse: $ touch c:/foo $ ls /cygdrive/c/foo foo As long as we support DOS paths as input, I have a hard time to see how to fix this, but maybe we can at least minimize the ambiguity somehow. I can't immediately think of anything. But is it really impossible to phase out DOS path support over a period of time? We could start with a HEADS-UP, asking for comments, then a deprecation announcement, then something like the old dosfilewarning option, then a more forceful warning that can't be turned off, and finally removal of support. This could be done over a period of several years (not sure how many). We could also put lines like # C:/ on /c type ntfs (binary,posix=0) into the default /etc/fstab. Ken
Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
On 10/23/2021 1:35 AM, Mark Geisert wrote: Corinna Vinschen wrote: Just to close this up prior to the 3.3.0 release... Given we never actually strived for 32<->64 bit interoperability, it's hard to argue why this should be different for the clipboard stuff. Running 32 and 64 bit Cygwin versions in parallel doesn't actually make much sense for most people anyway, unless they explicitely develop for 32 and 64 bit systems under Cygwin. From a productivity point of view there's no good reason to run more than one arch. So I agree with Ken here. It's probably not worth the trouble. Sorry, I've been sidetracked for a bit. I can agree with Ken too. The only circumstance I could think of where multiple internal format support might be useful (to non-developers) was some user hanging on to an older Cygwin because it was needed to support something else (s/w or h/w) old and non-upgradeable. Doesn't seem very likely at this point. I'll try to get the v2 patch out over this weekend. Same end-result for same environments as the v1 patch, but incorporating all the comments I received. I think Corinna was saying that the whole idea of making the 32-bit and 64-bit clipboards interoperable is not worth the trouble. To that end, does Jon's suggestion of /usr/include/sys/cygwin.h seem like the best location to define struct cygcb_t for use by both Cygwin and cygutils package? Thanks much, ..mark
Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
On 10/11/2021 2:13 AM, Mark Geisert wrote: It's just that after submitting the patch I realized that, if we really are going to support both Cygwin archs (x86_64 and i686), there is still the issue of different cygcb_t layouts between Cygwin versions being ignored. Specifically, the fhandler_clipboard::fstat routine can't tell which Cygwin environment has set the clipboard contents. My original patch takes care of 32-bit and 64-bit, providing both are running Cygwin >= 3.3.0 (presumably). What if it was a different version (pre 3.3.0) that set the contents? I wonder if this is worth the trouble. Right now we have a problem in which data written to /dev/clipboard in one arch can't be read in the other arch. The fix will appear in Cygwin 3.3.0. Do we really have to try to make the fix apply retroactively in case the user updates one arch but not the other? Ken
Re: [PATCH] Cygwin: pty: Fix handle leak regarding attach_mutex.
On 10/9/2021 8:49 PM, Takashi Yano wrote:
- If the process having master pty opened is forked, attach_mutex
fails to be closed when master is closed. This patch fixes the
issue.
---
winsup/cygwin/fhandler_console.cc | 2 +-
winsup/cygwin/fhandler_tty.cc | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_console.cc
b/winsup/cygwin/fhandler_console.cc
index ee862b17d..aee5e8284 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -57,7 +57,7 @@ fhandler_console::console_state NO_COPY
*fhandler_console::shared_console_info;
bool NO_COPY fhandler_console::invisible_console;
/* Mutex for AttachConsole()/FreeConsole() in fhandler_tty.cc */
-HANDLE NO_COPY attach_mutex;
+HANDLE attach_mutex;
static inline void
acquire_attach_mutex (DWORD t)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 823dabf73..f523dafed 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -57,7 +57,7 @@ struct pipe_reply {
};
extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */
-static LONG NO_COPY master_cnt = 0;
+static LONG master_cnt = 0;
inline static bool pcon_pid_alive (DWORD pid);
@@ -2042,10 +2042,10 @@ fhandler_pty_master::close ()
}
release_output_mutex ();
master_fwd_thread->terminate_thread ();
- if (InterlockedDecrement (_cnt) == 0)
- CloseHandle (attach_mutex);
}
}
+ if (InterlockedDecrement (_cnt) == 0)
+CloseHandle (attach_mutex);
/* Check if the last master handle has been closed. If so, set
input_available_event to wake up potentially waiting slaves. */
Pushed. Thanks.
Ken
Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
On 10/9/2021 10:29 AM, Jon Turney wrote:
On 07/10/2021 06:22, Mark Geisert wrote:
> The cygutils package has two programs, putclip and getclip, that also
> depend on the layout of the cygcb_t. At present they have duplicate
> defs of struct cygcb_t defined here as no Cygwin header provides it.
This struct should maybe be in sys/cygwin.h or similar, if it's expected to be
used in user-space as well.
Good idea.
On 09/10/2021 15:19, Ken Brown wrote:
On 10/8/2021 5:52 AM, Takashi Yano wrote:
How about simply just:
diff --git a/winsup/cygwin/fhandler_clipboard.cc
b/winsup/cygwin/fhandler_clipboard.cc
index ccdb295f3..d822f4fc4 100644
--- a/winsup/cygwin/fhandler_clipboard.cc
+++ b/winsup/cygwin/fhandler_clipboard.cc
@@ -28,9 +28,10 @@ static const WCHAR *CYGWIN_NATIVE =
L"CYGWIN_NATIVE_CLIPBOARD";
typedef struct
{
- timestruc_t timestamp;
- size_t len;
- char data[1];
+ uint64_t tv_sec;
+ uint64_t tv_nsec;
+ uint64_t len;
+ char data[1];
} cygcb_t;
The only problem with this is that it might leave readers scratching their
heads unless they look at the commit that introduced this. What
I think the solution to that is a "comment" like "we don't use 'struct timespec'
here as it's different size on different arches and that causes problem XYZ".
And the same for size_t.
Ken
Re: [PATCH] Cygwin: Make native clipboard layout same for 32- and 64-bit
On 10/8/2021 5:52 AM, Takashi Yano wrote:
How about simply just:
diff --git a/winsup/cygwin/fhandler_clipboard.cc
b/winsup/cygwin/fhandler_clipboard.cc
index ccdb295f3..d822f4fc4 100644
--- a/winsup/cygwin/fhandler_clipboard.cc
+++ b/winsup/cygwin/fhandler_clipboard.cc
@@ -28,9 +28,10 @@ static const WCHAR *CYGWIN_NATIVE =
L"CYGWIN_NATIVE_CLIPBOARD";
typedef struct
{
- timestruc_t timestamp;
- size_t len;
- char data[1];
+ uint64_t tv_sec;
+ uint64_t tv_nsec;
+ uint64_t len;
+ char data[1];
} cygcb_t;
The only problem with this is that it might leave readers scratching their heads
unless they look at the commit that introduced this. What about something like
the following, in which the code speaks for itself:
diff --git a/winsup/cygwin/fhandler_clipboard.cc
b/winsup/cygwin/fhandler_clipboard.cc
index ccdb295f3..028c00f1e 100644
--- a/winsup/cygwin/fhandler_clipboard.cc
+++ b/winsup/cygwin/fhandler_clipboard.cc
@@ -26,12 +26,26 @@ details. */
static const WCHAR *CYGWIN_NATIVE = L"CYGWIN_NATIVE_CLIPBOARD";
+#ifdef __x86_64__
typedef struct
{
timestruc_t timestamp;
size_t len;
char data[1];
} cygcb_t;
+#else
+/* Use same layout. */
+typedef struct
+{
+ struct
+ {
+int64_t tv_sec;
+int64_t tv_nsec;
+ }timestamp;
+ uint64_t len;
+ char data[1];
+} cygcb_t;
+#endif
fhandler_dev_clipboard::fhandler_dev_clipboard ()
: fhandler_base (), pos (0), membuffer (NULL), msize (0)
@@ -74,7 +88,14 @@ fhandler_dev_clipboard::set_clipboard (const void *buf,
size_t len)
}
clipbuf = (cygcb_t *) GlobalLock (hmem);
+#ifdef __x86_64__
clock_gettime (CLOCK_REALTIME, >timestamp);
+#else
+ timestruc_t ts;
+ clock_gettime (CLOCK_REALTIME, );
+ clipbuf->timestamp->tv_sec = ts.tv_sec;
+ clipbuf->timestamp->tv_nsec = ts.tv_nsec;
+#endif
clipbuf->len = len;
memcpy (clipbuf->data, buf, len);
@@ -179,7 +200,14 @@ fhandler_dev_clipboard::fstat (struct stat *buf)
&& (hglb = GetClipboardData (format))
&& (clipbuf = (cygcb_t *) GlobalLock (hglb)))
{
+#ifdef __x86_64__
buf->st_atim = buf->st_mtim = clipbuf->timestamp;
+#else
+ timestruc_t ts;
+ ts.tv_sec = clipbuf->timestamp->tv_sec;
+ ts.tv_nsec = clipbuf->timestamp->tv_nsec;
+ buf->st_atim = buf->st_mtim = ts;
+#endif
buf->st_size = clipbuf->len;
GlobalUnlock (hglb);
}
Ken
Re: [PATCH] Cygwin: pty: Fix master closing error regarding attach_mutex.
On 10/8/2021 12:28 PM, Takashi Yano wrote:
- If two or more pty masters are opened in a process, closing master
causes error when closing attach_mutex. This patch fixes the issue.
Addresses:
https://cygwin.com/pipermail/cygwin-developers/2021-October/012418.html
---
winsup/cygwin/fhandler_tty.cc | 7 +--
winsup/cygwin/release/3.3.0 | 3 +++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 05fe5348a..823dabf73 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -57,6 +57,7 @@ struct pipe_reply {
};
extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */
+static LONG NO_COPY master_cnt = 0;
inline static bool pcon_pid_alive (DWORD pid);
@@ -2041,7 +2042,8 @@ fhandler_pty_master::close ()
}
release_output_mutex ();
master_fwd_thread->terminate_thread ();
- CloseHandle (attach_mutex);
+ if (InterlockedDecrement (_cnt) == 0)
+ CloseHandle (attach_mutex);
}
}
@@ -2876,7 +2878,8 @@ fhandler_pty_master::setup ()
if (!(pcon_mutex = CreateMutex (, FALSE, buf)))
goto err;
- attach_mutex = CreateMutex (, FALSE, NULL);
+ if (InterlockedIncrement (_cnt) == 1)
+attach_mutex = CreateMutex (, FALSE, NULL);
/* Create master control pipe which allows the master to duplicate
the pty pipe handles to processes which deserve it. */
diff --git a/winsup/cygwin/release/3.3.0 b/winsup/cygwin/release/3.3.0
index 2f7340ac5..2df81a4ae 100644
--- a/winsup/cygwin/release/3.3.0
+++ b/winsup/cygwin/release/3.3.0
@@ -71,3 +71,6 @@ Bug Fixes
in ps(1) output.
Addresses: https://cygwin.com/pipermail/cygwin/2021-July/248998.html
https://cygwin.com/pipermail/cygwin/2021-August/249124.html
+
+- Fix pty master closing error regarding attach_mutex.
+ Addresses:
https://cygwin.com/pipermail/cygwin-developers/2021-October/012418.html
Pushed. Thanks.
Ken
[PATCH 2/2] Cygwin: getifaddrs: don't return a zero IPv4 address
Patch attached.
>From f869ec6f96e16f09be7098740bc21c0c39544fd4 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Mon, 26 Jul 2021 10:27:53 -0400
Subject: [PATCH 2/2] Cygwin: getifaddrs: don't return a zero IPv4 address
If an interface is disconnected, net.cc:get_ifs tries to fetch IPv4
addresses from the registry. If it fails, it currently returns
pointers to sockaddr structs with zero address. Return a NULL pointer
instead, to signal the caller of getifaddrs that we do not have a
valid struct sockaddr.
Partially addresses: https://cygwin.com/pipermail/cygwin/2021-July/248970.html
---
winsup/cygwin/net.cc | 29 +++--
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/net.cc b/winsup/cygwin/net.cc
index 67dd7fc04..5bdc8709f 100644
--- a/winsup/cygwin/net.cc
+++ b/winsup/cygwin/net.cc
@@ -1595,6 +1595,7 @@ static void
get_ipv4fromreg (struct ifall *ifp, const char *name, DWORD idx)
{
WCHAR regkey[256], *c;
+ bool got_addr = false, got_mask = false;
c = wcpcpy (regkey, L"Tcpip\\Parameters\\Interfaces\\");
sys_mbstowcs (c, 220, name);
@@ -1637,9 +1638,15 @@ get_ipv4fromreg (struct ifall *ifp, const char *name,
DWORD idx)
if (dhcp)
{
if (udipa.Buffer)
- inet_uton (udipa.Buffer, >sin_addr);
+ {
+ inet_uton (udipa.Buffer, >sin_addr);
+ got_addr = true;
+ }
if (udsub.Buffer)
- inet_uton (udsub.Buffer, >sin_addr);
+ {
+ inet_uton (udsub.Buffer, >sin_addr);
+ got_mask = true;
+ }
}
else
{
@@ -1649,7 +1656,10 @@ get_ipv4fromreg (struct ifall *ifp, const char *name,
DWORD idx)
c += wcslen (c) + 1)
ifs++;
if (*c)
- inet_uton (c, >sin_addr);
+ {
+ inet_uton (c, >sin_addr);
+ got_addr = true;
+ }
}
if (usub.Buffer)
{
@@ -1657,9 +1667,16 @@ get_ipv4fromreg (struct ifall *ifp, const char *name,
DWORD idx)
c += wcslen (c) + 1)
ifs++;
if (*c)
- inet_uton (c, >sin_addr);
+ {
+ inet_uton (c, >sin_addr);
+ got_mask = true;
+ }
}
}
+ if (got_addr)
+ ifp->ifa_ifa.ifa_addr = (struct sockaddr *) addr;
+ if (got_mask)
+ ifp->ifa_ifa.ifa_netmask = (struct sockaddr *) mask;
if (ifp->ifa_ifa.ifa_flags & IFF_BROADCAST)
brdc->sin_addr.s_addr = (addr->sin_addr.s_addr
& mask->sin_addr.s_addr)
@@ -1800,13 +1817,13 @@ get_ifs (ULONG family)
ifp->ifa_ifa.ifa_flags |= IFF_BROADCAST;
/* Address */
ifp->ifa_addr.ss_family = AF_INET;
- ifp->ifa_ifa.ifa_addr = (struct sockaddr *) >ifa_addr;
+ ifp->ifa_ifa.ifa_addr = NULL;
/* Broadcast/Destination address */
ifp->ifa_brddstaddr.ss_family = AF_INET;
ifp->ifa_ifa.ifa_dstaddr = NULL;
/* Netmask */
ifp->ifa_netmask.ss_family = AF_INET;
- ifp->ifa_ifa.ifa_netmask = (struct sockaddr *) >ifa_netmask;
+ ifp->ifa_ifa.ifa_netmask = NULL;
/* Try to fetch real IPv4 address information from registry. */
get_ipv4fromreg (ifp, pap->AdapterName, idx);
/* Hardware address */
--
2.32.0
[PATCH 1/2] Cygwin: getifaddrs: fix address family for IPv6 netmasks
Patch attached. >From 49ff12b47d9d71960fd7b39846b302b8031affa4 Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Mon, 26 Jul 2021 08:59:09 -0400 Subject: [PATCH 1/2] Cygwin: getifaddrs: fix address family for IPv6 netmasks The code in net.cc:get_ifs that sets the netmask omitted setting the address family in the IPv6 case. Fix this by setting it to AF_INET6. Partially addresses: https://cygwin.com/pipermail/cygwin/2021-July/248970.html --- winsup/cygwin/net.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/winsup/cygwin/net.cc b/winsup/cygwin/net.cc index cec0a70cc..67dd7fc04 100644 --- a/winsup/cygwin/net.cc +++ b/winsup/cygwin/net.cc @@ -1869,6 +1869,7 @@ get_ifs (ULONG family) if (prefix < 32) if_sin6->sin6_addr.s6_addr32[cnt] <<= 32 - prefix; } + if_sin6->sin6_family = AF_INET6; break; } ifp->ifa_ifa.ifa_netmask = (struct sockaddr *) >ifa_netmask; -- 2.32.0
[PATCH 0/2] Fix getifaddrs problems
The two patches in this series fix the two problems reported in https://cygwin.com/pipermail/cygwin/2021-July/248970.html As I indicated in that message, I'm not 100% sure of the second patch. Ken Ken Brown (2): Cygwin: getifaddrs: fix address family for IPv6 netmasks Cygwin: getifaddrs: don't return a zero IPv4 address winsup/cygwin/net.cc | 30 -- 1 file changed, 24 insertions(+), 6 deletions(-) -- 2.32.0 P.S. I'm sending the patches as attachments in the next two messages because my SMTP server uses an authentication method that is not compatible with git send-email. If this gets to be annoying, I have a different email account that I could start using for patches.
[PATCH] Cygwin: cfsetspeed: allow speed to be a numerical baud rate
The attached patch addresses
https://cygwin.com/pipermail/cygwin/2021-July/248887.html
I don't really understand the GPL issue, but I hope it's OK.
Ken
>From 0321ecd99050ad702a528797af48ea4d01531508 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sun, 11 Jul 2021 07:04:58 -0400
Subject: [PATCH] Cygwin: cfsetspeed: allow speed to be a numerical baud rate
The Linux man page for cfsetspeed(3) specifies that the speed argument
must be one of the constants Bnnn (e.g., B9600) defined in termios.h.
But Linux in fact allows the speed to be the numerical baud rate
(e.g., 9600). For consistency with Linux, we now do the same.
Addresses: https://cygwin.com/pipermail/cygwin/2021-July/248887.html
---
winsup/cygwin/release/3.2.1 | 4 +++
winsup/cygwin/termios.cc| 59 +
winsup/doc/new-features.xml | 11 +--
3 files changed, 71 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/release/3.2.1 b/winsup/cygwin/release/3.2.1
index 6ebe68fa6..99c65ce30 100644
--- a/winsup/cygwin/release/3.2.1
+++ b/winsup/cygwin/release/3.2.1
@@ -5,6 +5,10 @@ What's new:
What changed:
-
+- The speed argument to cfsetspeed(3) can now be a numerical baud rate
+ rather than a Bnnn constant, as on Linux.
+ Addresses: https://cygwin.com/pipermail/cygwin/2021-July/248887.html
+
Bug Fixes
-
diff --git a/winsup/cygwin/termios.cc b/winsup/cygwin/termios.cc
index b29a64af2..ee9cd23b7 100644
--- a/winsup/cygwin/termios.cc
+++ b/winsup/cygwin/termios.cc
@@ -325,12 +325,71 @@ cfsetispeed (struct termios *in_tp, speed_t speed)
return res;
}
+struct speed_struct
+{
+ speed_t value;
+ speed_t internal;
+};
+
+static const struct speed_struct speeds[] =
+ {
+{ 0, B0 },
+{ 50, B50 },
+{ 75, B75 },
+{ 110, B110 },
+{ 134, B134 },
+{ 150, B150 },
+{ 200, B200 },
+{ 300, B300 },
+{ 600, B600 },
+{ 1200, B1200 },
+{ 1800, B1800 },
+{ 2400, B2400 },
+{ 4800, B4800 },
+{ 9600, B9600 },
+{ 19200, B19200 },
+{ 38400, B38400 },
+{ 57600, B57600 },
+{ 115200, B115200 },
+{ 128000, B128000 },
+{ 230400, B230400 },
+{ 256000, B256000 },
+{ 460800, B460800 },
+{ 50, B50 },
+{ 576000, B576000 },
+{ 921600, B921600 },
+{ 100, B100 },
+{ 1152000, B1152000 },
+{ 150, B150 },
+{ 200, B200 },
+{ 250, B250 },
+{ 300, B300 },
+ };
+
+/* Given a numerical baud rate (e.g., 9600), convert it to a Bnnn
+ constant (e.g., B9600). */
+static speed_t
+convert_speed (speed_t speed)
+{
+ for (size_t i = 0; i < sizeof speeds / sizeof speeds[0]; i++)
+{
+ if (speed == speeds[i].internal)
+ return speed;
+ else if (speed == speeds[i].value)
+ return speeds[i].internal;
+}
+ return speed;
+}
+
/* cfsetspeed: 4.4BSD */
+/* Following Linux (undocumented), allow speed to be a numerical baud rate. */
extern "C" int
cfsetspeed (struct termios *in_tp, speed_t speed)
{
struct termios *tp = __tonew_termios (in_tp);
int res;
+
+ speed = convert_speed (speed);
/* errors come only from unsupported baud rates, so setspeed() would return
identical results in both calls */
if ((res = setspeed (tp->c_ospeed, speed)) == 0)
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 5ec36e409..b58872935 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -71,9 +71,14 @@ facl(2) now fails with EBADF on a file opened with O_PATH.
-- Allow to start Windows Store executables via their "app execution
- aliases". Handle these aliases (which are special reparse points)
- as symlinks to the actual executables.
+Allow to start Windows Store executables via their "app execution
+aliases". Handle these aliases (which are special reparse points)
+as symlinks to the actual executables.
+
+
+
+The speed argument to cfsetspeed(3) can now be a numerical baud rate
+rather than a Bnnn constant, as on Linux.
--
2.32.0
[PATCH] Cygwin: fhandler_mqueue::mq_open: fix typo
Patch attached.
Sorry for using an attachment, but my smtp server just started using 2-factor
authorization, and I haven't figured out how to make it work with git-send-email.
Ken
>From dfe5988f961ff97d283a9c460e75499db168163a Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Wed, 26 May 2021 12:48:58 -0400
Subject: [PATCH] Cygwin: fhandler_mqueue::mq_open: fix typo
---
winsup/cygwin/fhandler_mqueue.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler_mqueue.cc b/winsup/cygwin/fhandler_mqueue.cc
index 745c80643..d81880cba 100644
--- a/winsup/cygwin/fhandler_mqueue.cc
+++ b/winsup/cygwin/fhandler_mqueue.cc
@@ -117,7 +117,7 @@ exists:
if (status != STATUS_SHARING_VIOLATION)
{
__seterrno_from_nt_status (status);
- return -1;
+ return 0;
}
Sleep (100L);
}
--
2.31.1
Re: [PATCH] Use automake (v5)
On 4/27/2021 12:52 PM, Ken Brown wrote: On 4/27/2021 11:50 AM, Jon Turney wrote: On 20/04/2021 21:13, Jon Turney wrote: For ease of reviewing, this patch doesn't contain changes to generated files which would be made by running ./autogen.sh. I pushed this patch. If you have an existing build directory, while you might get away with invoking 'make' at the top-level, I would recommend blowing it away and configuring again. I'm confused about how the generated files are going to get regenerated when necessary. I see calls to autogen (which I guess is /usr/bin/autogen.exe from the autogen package) in the Makefiles, but I don't see any calls to autogen.sh. Is the latter no longer needed? Oh, never mind. The Makefiles just call autoconf, etc., as needed. Ken
Re: [PATCH] Use automake (v5)
On 4/27/2021 11:50 AM, Jon Turney wrote: On 20/04/2021 21:13, Jon Turney wrote: For ease of reviewing, this patch doesn't contain changes to generated files which would be made by running ./autogen.sh. I pushed this patch. If you have an existing build directory, while you might get away with invoking 'make' at the top-level, I would recommend blowing it away and configuring again. I'm confused about how the generated files are going to get regenerated when necessary. I see calls to autogen (which I guess is /usr/bin/autogen.exe from the autogen package) in the Makefiles, but I don't see any calls to autogen.sh. Is the latter no longer needed? Ken
Re: [PATCH] Cygwin: connect: implement resetting a connected DGRAM socket
On 4/27/2021 9:19 AM, Corinna Vinschen wrote: On Apr 26 15:37, Ken Brown wrote: Following POSIX and Linux, allow a connected DGRAM socket's connection to be reset (so that the socket becomes unconnected). This is done by calling connect and specifing an address whose family is AF_UNSPEC. --- winsup/cygwin/fhandler_socket_inet.cc | 21 -- winsup/cygwin/fhandler_socket_local.cc | 30 +- winsup/cygwin/fhandler_socket_unix.cc | 7 ++ winsup/cygwin/release/3.2.1| 3 +++ winsup/doc/new-features.xml| 6 ++ 5 files changed, 60 insertions(+), 7 deletions(-) LGTM. --- a/winsup/cygwin/release/3.2.1 +++ b/winsup/cygwin/release/3.2.1 @@ -1,6 +1,9 @@ What's new: --- +- A connected datagram socket can now have its connection reset. As + specified by POSIX and Linux, this is done by calling connect(2) + with an address structure whose family is AF_UNSPEC. Isn't that just a bug, in theory? I was thinking of it as a feature that hadn't been implemented yet. But on second thought, I agree with you. I'll change that. Ken
[PATCH] Cygwin: connect: implement resetting a connected DGRAM socket
Following POSIX and Linux, allow a connected DGRAM socket's connection
to be reset (so that the socket becomes unconnected). This is done by
calling connect and specifing an address whose family is AF_UNSPEC.
---
winsup/cygwin/fhandler_socket_inet.cc | 21 --
winsup/cygwin/fhandler_socket_local.cc | 30 +-
winsup/cygwin/fhandler_socket_unix.cc | 7 ++
winsup/cygwin/release/3.2.1| 3 +++
winsup/doc/new-features.xml| 6 ++
5 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/winsup/cygwin/fhandler_socket_inet.cc
b/winsup/cygwin/fhandler_socket_inet.cc
index f6bb8c503..30eab4099 100644
--- a/winsup/cygwin/fhandler_socket_inet.cc
+++ b/winsup/cygwin/fhandler_socket_inet.cc
@@ -781,8 +781,20 @@ int
fhandler_socket_inet::connect (const struct sockaddr *name, int namelen)
{
struct sockaddr_storage sst;
+ bool reset = (name->sa_family == AF_UNSPEC
+ && get_socket_type () == SOCK_DGRAM);
- if (get_inet_addr_inet (name, namelen, , ) == SOCKET_ERROR)
+ if (reset)
+{
+ if (connect_state () == unconnected)
+ return 0;
+ /* To reset a connected DGRAM socket, call Winsock's connect
+function with the address member of the sockaddr structure
+filled with zeroes. */
+ memset (, 0, sizeof sst);
+ sst.ss_family = get_addr_family ();
+}
+ else if (get_inet_addr_inet (name, namelen, , ) == SOCKET_ERROR)
return SOCKET_ERROR;
/* Initialize connect state to "connect_pending". In the SOCK_STREAM
@@ -804,7 +816,12 @@ fhandler_socket_inet::connect (const struct sockaddr
*name, int namelen)
int res = ::connect (get_socket (), (struct sockaddr *) , namelen);
if (!res)
-connect_state (connected);
+{
+ if (reset)
+ connect_state (unconnected);
+ else
+ connect_state (connected);
+}
else if (!is_nonblocking ()
&& res == SOCKET_ERROR
&& WSAGetLastError () == WSAEWOULDBLOCK)
diff --git a/winsup/cygwin/fhandler_socket_local.cc
b/winsup/cygwin/fhandler_socket_local.cc
index 1c8d48b58..bd4081622 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -894,19 +894,34 @@ fhandler_socket_local::connect (const struct sockaddr
*name, int namelen)
{
struct sockaddr_storage sst;
int type = 0;
+ bool reset = (name->sa_family == AF_UNSPEC
+ && get_socket_type () == SOCK_DGRAM);
- if (get_inet_addr_local (name, namelen, , , ,
connect_secret)
- == SOCKET_ERROR)
+ if (reset)
+{
+ if (connect_state () == unconnected)
+ return 0;
+ /* To reset a connected DGRAM socket, call Winsock's connect
+function with the address member of the sockaddr structure
+filled with zeroes. */
+ memset (, 0, sizeof sst);
+ sst.ss_family = get_addr_family ();
+}
+ else if (get_inet_addr_local (name, namelen, , , ,
+ connect_secret) == SOCKET_ERROR)
return SOCKET_ERROR;
- if (get_socket_type () != type)
+ if (get_socket_type () != type && !reset)
{
WSASetLastError (WSAEPROTOTYPE);
set_winsock_errno ();
return SOCKET_ERROR;
}
- set_peer_sun_path (name->sa_data);
+ if (reset)
+set_peer_sun_path (NULL);
+ else
+set_peer_sun_path (name->sa_data);
/* Don't move af_local_set_cred into af_local_connect which may be called
via select, possibly running under another identity. Call early here,
@@ -933,7 +948,12 @@ fhandler_socket_local::connect (const struct sockaddr
*name, int namelen)
int res = ::connect (get_socket (), (struct sockaddr *) , namelen);
if (!res)
-connect_state (connected);
+{
+ if (reset)
+ connect_state (unconnected);
+ else
+ connect_state (connected);
+}
else if (!is_nonblocking ()
&& res == SOCKET_ERROR
&& WSAGetLastError () == WSAEWOULDBLOCK)
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index 252bcd9a9..a2428e952 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -1696,6 +1696,13 @@ fhandler_socket_unix::connect (const struct sockaddr
*name, int namelen)
conn_unlock ();
return -1;
}
+ if (name->sa_family == AF_UNSPEC && get_socket_type () == SOCK_DGRAM)
+{
+ connect_state (unconnected);
+ peer_sun_path (NULL);
+ conn_unlock ();
+ return 0;
+}
connect_state (connect_pending);
conn_unlock ();
/* Check validity of name */
diff --git a/winsup/cygwin/release/3.2.1 b/winsup/cygwin/release/3.2.1
index 7662c7114..9edf509bb 100644
--- a/winsup/cygwin/release/3.2.1
+++ b/winsup/cygwin/release/3.2.1
@@ -1,6 +1,9 @@
What's new:
---
+- A connected datagram socket can now have its connection reset. As
+ specified by POSIX and Linux, this is done by calling connect(2)
+ with an address structure whose
[PATCH] Cygwin: connect: set connect state for DGRAM sockets
When connect is called on a DGRAM socket, the call to Winsock's connect can immediately return successfully rather than failing with WSAEWOULDBLOCK. Set the connect state to "connected" in this case. Previously the connect state remained "connect_pending" after the successful connection. --- winsup/cygwin/fhandler_socket_inet.cc | 19 +++ winsup/cygwin/fhandler_socket_local.cc | 19 +++ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/winsup/cygwin/fhandler_socket_inet.cc b/winsup/cygwin/fhandler_socket_inet.cc index 4ecb31a27..f6bb8c503 100644 --- a/winsup/cygwin/fhandler_socket_inet.cc +++ b/winsup/cygwin/fhandler_socket_inet.cc @@ -785,11 +785,13 @@ fhandler_socket_inet::connect (const struct sockaddr *name, int namelen) if (get_inet_addr_inet (name, namelen, , ) == SOCKET_ERROR) return SOCKET_ERROR; - /* Initialize connect state to "connect_pending". State is ultimately set - to "connected" or "connect_failed" in wait_for_events when the FD_CONNECT - event occurs. Note that the underlying OS sockets are always non-blocking - and a successfully initiated non-blocking Winsock connect always returns - WSAEWOULDBLOCK. Thus it's safe to rely on event handling. + /* Initialize connect state to "connect_pending". In the SOCK_STREAM + case, the state is ultimately set to "connected" or "connect_failed" in + wait_for_events when the FD_CONNECT event occurs. Note that the + underlying OS sockets are always non-blocking in this case and a + successfully initiated non-blocking Winsock connect always returns + WSAEWOULDBLOCK. Thus it's safe to rely on event handling. For DGRAM + sockets, however, connect can return immediately. Check for either unconnected or connect_failed since in both cases it's allowed to retry connecting the socket. It's also ok (albeit ugly) to @@ -801,7 +803,9 @@ fhandler_socket_inet::connect (const struct sockaddr *name, int namelen) connect_state (connect_pending); int res = ::connect (get_socket (), (struct sockaddr *) , namelen); - if (!is_nonblocking () + if (!res) +connect_state (connected); + else if (!is_nonblocking () && res == SOCKET_ERROR && WSAGetLastError () == WSAEWOULDBLOCK) res = wait_for_events (FD_CONNECT | FD_CLOSE, 0); @@ -824,8 +828,7 @@ fhandler_socket_inet::connect (const struct sockaddr *name, int namelen) Convert to POSIX/Linux compliant EISCONN. */ else if (err == WSAEINVAL && connect_state () == listener) WSASetLastError (WSAEISCONN); - /* Any other error except WSAEALREADY during connect_pending means the - connect failed. */ + /* Any other error except WSAEALREADY means the connect failed. */ else if (connect_state () == connect_pending && err != WSAEALREADY) connect_state (connect_failed); set_winsock_errno (); diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc index ad7dd0a98..1c8d48b58 100644 --- a/winsup/cygwin/fhandler_socket_local.cc +++ b/winsup/cygwin/fhandler_socket_local.cc @@ -914,11 +914,13 @@ fhandler_socket_local::connect (const struct sockaddr *name, int namelen) if (get_socket_type () == SOCK_STREAM) af_local_set_cred (); - /* Initialize connect state to "connect_pending". State is ultimately set - to "connected" or "connect_failed" in wait_for_events when the FD_CONNECT - event occurs. Note that the underlying OS sockets are always non-blocking - and a successfully initiated non-blocking Winsock connect always returns - WSAEWOULDBLOCK. Thus it's safe to rely on event handling. + /* Initialize connect state to "connect_pending". In the SOCK_STREAM + case, the state is ultimately set to "connected" or "connect_failed" in + wait_for_events when the FD_CONNECT event occurs. Note that the + underlying OS sockets are always non-blocking in this case and a + successfully initiated non-blocking Winsock connect always returns + WSAEWOULDBLOCK. Thus it's safe to rely on event handling. For DGRAM + sockets, however, connect can return immediately. Check for either unconnected or connect_failed since in both cases it's allowed to retry connecting the socket. It's also ok (albeit ugly) to @@ -930,7 +932,9 @@ fhandler_socket_local::connect (const struct sockaddr *name, int namelen) connect_state (connect_pending); int res = ::connect (get_socket (), (struct sockaddr *) , namelen); - if (!is_nonblocking () + if (!res) +connect_state (connected); + else if (!is_nonblocking () && res == SOCKET_ERROR && WSAGetLastError () == WSAEWOULDBLOCK) res = wait_for_events (FD_CONNECT | FD_CLOSE, 0); @@ -953,8 +957,7 @@ fhandler_socket_local::connect (const struct sockaddr *name, int namelen) Convert to POSIX/Linux compliant EISCONN. */ else if (err == WSAEINVAL && connect_state
Re: [PATCH 1/2] Treat Windows Store's "app execution aliases" as symbolic links
On 3/24/2021 2:55 PM, Hans-Bernhard Bröker wrote: Am 23.03.2021 um 10:30 schrieb Corinna Vinschen via Cygwin-patches: > On Mar 22 22:54, Hans-Bernhard Bröker wrote: >> Am 22.03.2021 um 16:22 schrieb Johannes Schindelin: >>> One of those under-documented reparse point types is the WSL symbolic >>> link, which you will notice are supported in Cygwin, removing quite some >>> sway from your argument... >> >> I notice no such thing right now, running the currently available release >> version 3.1.7: >> >> stat: cannot stat '//wsl$/Debian/home/hbbro/link_to_a': Input/output error > > What type of WSL symlink is that? It's what WSL Debian creates when I 'ln -s' inside its own filesystem. Windows' own "dir" command shows it as 22.03.2021 22:34 link_to_a [...] But it cannot do anything else with it. Even fsutil doesn't work on that thing: C:\prg\test>fsutil reparsePoint query \\wsl$\Debian\home\hbbro Fehler: Unzulässige Funktion. Are you running WSL1 or WSL2? I have WSL1, and the stat command such as the one you tried fails in the same way as yours. Nevertheless, a symlink created under WSL is indeed recognized as such by Cygwin. I verified this as follows: 1. Within WSL, $ ln -s foo mysymlink $ cp -a mysymlink /mnt/c/cygwin64/tmp 2. Within Cygwin, $ stat /tmp/mysymlink File: /tmp/mysymlink -> foo Size: 3 Blocks: 0 IO Block: 65536 symbolic link Device: 74d6767bh/1960212091d Inode: 25614222880728371 Links: 1 Access: (0777/lrwxrwxrwx) Uid: (197609/ kbrown) Gid: (197121/None) Access: 2021-03-24 16:25:50.729219700 -0400 Modify: 2021-03-24 16:25:50.729219700 -0400 Change: 2021-03-24 16:27:13.979376200 -0400 Birth: 2021-03-24 16:27:13.979376200 -0400 3. I then ran the stat command under gdb with a breakpoint at check_reparse_point_target and verified that Cygwin recognized /tmp/mysymlink as a WSL symlink (IO_REPARSE_TAG_LX_SYMLINK). Someone with WSL2 should try a similar experiment to make sure that the symlink representation as a reparse point hasn't changed. As to the failure of the stat command that you tried, I suspect it is related to the '\\wsl$' magic rather than anything to do with the symlink itself. If you run that stat command under strace, you'll see that Cygwin calls NtCreateFile (\??\UNC\wsl$\...), which succeeds, and then calls NtFsControlFile(FSCTL_GET_REPARSE_POINT), which fails with STATUS_NOT_IMPLEMENTED. Ken
Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().
[Still CC Marco]
On 3/22/2021 7:43 AM, Corinna Vinschen via Cygwin-patches wrote:
[CC Marco]
On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
On Sun, 21 Mar 2021 17:44:27 +0900
Takashi Yano wrote:
On Sun, 21 Mar 2021 13:01:24 +0900
Takashi Yano wrote:
Takashi Yano (2):
Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.
Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.
winsup/cygwin/fhandler_tty.cc | 19 +++
winsup/cygwin/syscalls.cc | 13 -
2 files changed, 31 insertions(+), 1 deletion(-)
I submitted these patches, however, I still wonder if we really
need these patches. I cannot imagine the situation where handle
itself is needed rather than file descriptor.
However, following cygwin apps/dlls call _get_osfhandle():
ccmake.exe
cmake.exe
cpack.exe
ctest.exe
ddrescue.exe
And also, following cygwin apps/dlls call GetStdHandle():
ccmake.exe
cmake.exe
cpack.exe
ctest.exe
run.exe
cygusb0.dll
tk86.dll
in my installation.
Therefore, some of these apps/dlls may need these patches...
I looked into cmake source and found the patch exactly for
this issue. Therefore, it seems better to fix this.
/* Get the Windows handle for a FILE stream. */
static HANDLE kwsysTerminalGetStreamHandle(FILE* stream)
{
/* Get the C-library file descriptor from the stream. */
int fd = fileno(stream);
# if defined(__CYGWIN__)
/* Cygwin seems to have an extra pipe level. If the file descriptor
corresponds to stdout or stderr then obtain the matching windows
handle directly. */
if (fd == fileno(stdout)) {
return GetStdHandle(STD_OUTPUT_HANDLE);
} else if (fd == fileno(stderr)) {
return GetStdHandle(STD_ERROR_HANDLE);
}
# endif
/* Get the underlying Windows handle for the descriptor. */
return (HANDLE)_get_osfhandle(fd);
}
Why on earth is cmake using Windows functions on Cygwin at all???
It's not as if it actually requires Windows functionality on our
platform.
Out of curiosity, I took a quick glance at the cmake code. It appears that this
code is designed to support running cmake in a Console. I don't think that
should be needed any more, if it ever was.
Marco, any input? Any chance to drop this Windows stuff from the Cygwin
code path in cmake?
I think the following might suffice (untested):
--- a/Source/kwsys/Terminal.c
+++ b/Source/kwsys/Terminal.c
@@ -10,7 +10,7 @@
#endif
/* Configure support for this platform. */
-#if defined(_WIN32) || defined(__CYGWIN__)
+#if defined(_WIN32)
# define KWSYS_TERMINAL_SUPPORT_CONSOLE
#endif
#if !defined(_WIN32)
Ken
Re: [PATCH] winsup/doc/dll.xml: update MinGW/.org to MinGW-w64/.org
On 3/8/2021 2:09 PM, Achim Gratz wrote: Brian Inglis writes: It's normally a merge conflict which will not be satisfied by regular commands to restore the working files to upstream. So you're pulling on an unclean work tree? That's a no-no, either keep your changes on a separate branch (that you can rebase or merge later) or stash them away for the pull. As Corinna said, if you're prepared to lose any local changes then git reset --hard will do that. But you should be sure you really didn't want any of your unfinished business around any more. If the unfinished business consists of local commits that haven't yet been applied upstream, then I typically do the following: git fetch # Find out if upstream has changed since my last pull. If so... git format-patch -n # save n local commits git reset --hard origin/master git am 00* # reapply my local commits This assumes I've been too lazy to work on a separate branch, which is often the case for small changes. Ken
[PATCH 7/7] Cygwin: simplify linkat with AT_EMPTY_PATH
linkat(olddirfd, oldpath, oldname, newdirfd, newname, AT_EMPTY_PATH)
is supposed to create a link to the file referenced by olddirfd if
oldname is the empty string. Currently this is done via the /proc
filesystem by converting the call to
linkat(AT_FDCWD, "/proc/self/fd/", newdirfd, newname,
AT_SYMLINK_FOLLOW),
which ultimately leads to a call to the appropriate fhandler's link
method. Simplify this by using cygheap_fdget to obtain the fhandler
directly.
---
winsup/cygwin/syscalls.cc | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 460fe6801..6ba4f10f7 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4962,6 +4962,8 @@ linkat (int olddirfd, const char *oldpathname,
int flags)
{
tmp_pathbuf tp;
+ fhandler_base *fh = NULL;
+
__try
{
if (flags & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH))
@@ -4970,21 +4972,25 @@ linkat (int olddirfd, const char *oldpathname,
__leave;
}
char *oldpath = tp.c_get ();
- /* AT_EMPTY_PATH with an empty oldpathname is equivalent to
-
- linkat(AT_FDCWD, "/proc/self/fd/", newdirfd,
- newname, AT_SYMLINK_FOLLOW);
-
-Convert the request accordingly. */
if ((flags & AT_EMPTY_PATH) && oldpathname && oldpathname[0] == '\0')
{
+ /* Operate directly on olddirfd, which can be anything
+except a directory. */
if (olddirfd == AT_FDCWD)
{
set_errno (EPERM);
__leave;
}
- __small_sprintf (oldpath, "/proc/%d/fd/%d", myself->pid, olddirfd);
- flags = AT_SYMLINK_FOLLOW;
+ cygheap_fdget cfd (olddirfd);
+ if (cfd < 0)
+ __leave;
+ if (cfd->pc.isdir ())
+ {
+ set_errno (EPERM);
+ __leave;
+ }
+ fh = cfd;
+ flags = 0;/* In case AT_SYMLINK_FOLLOW was set. */
}
else if (gen_full_path_at (oldpath, olddirfd, oldpathname))
__leave;
@@ -5003,6 +5009,8 @@ linkat (int olddirfd, const char *oldpathname,
}
strcpy (oldpath, old_name.get_posix ());
}
+ if (fh)
+ return fh->link (newpath);
return link (oldpath, newpath);
}
__except (EFAULT) {}
--
2.30.0
[PATCH 6/7] Cygwin: fix linkat(2) on sockets that are not socket files
If linkat(2) is called with AT_EMPTY_PATH on an AF_LOCAL or
AF_UNIX socket that is not a socket file, the current code calls
fhandler_disk_file::link in most cases. The latter expects to be
operating on a disk file and uses the socket's io_handle, which
is not a file handle.
Fix this by calling fhandler_disk_file::link only if the
fhandler_socket object is a file (determined by testing
dev().isfs()).
Also fix the case of a socket file opened with O_PATH by setting
the fhandler_disk_file's io_handle.
---
winsup/cygwin/fhandler_socket_local.cc | 7 ++-
winsup/cygwin/fhandler_socket_unix.cc | 9 ++---
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_socket_local.cc
b/winsup/cygwin/fhandler_socket_local.cc
index 22586c0dd..ad7dd0a98 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -750,9 +750,14 @@ fhandler_socket_local::facl (int cmd, int nentries,
aclent_t *aclbufp)
int
fhandler_socket_local::link (const char *newpath)
{
- if (get_sun_path () && get_sun_path ()[0] == '\0')
+ if (!dev ().isfs ())
+/* linkat w/ AT_EMPTY_PATH called on a socket not opened w/ O_PATH. */
return fhandler_socket_wsock::link (newpath);
+ /* link on a socket file or linkat w/ AT_EMPTY_PATH called on a
+ socket opened w/ O_PATH. */
fhandler_disk_file fh (pc);
+ if (get_flags () & O_PATH)
+fh.set_handle (get_handle ());
return fh.link (newpath);
}
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index fae07367d..252bcd9a9 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -2421,11 +2421,14 @@ fhandler_socket_unix::facl (int cmd, int nentries,
aclent_t *aclbufp)
int
fhandler_socket_unix::link (const char *newpath)
{
- if (sun_path ()
- && (sun_path ()->un_len <= (socklen_t) sizeof (sa_family_t)
- || sun_path ()->un.sun_path[0] == '\0'))
+ if (!dev ().isfs ())
+/* linkat w/ AT_EMPTY_PATH called on a socket not opened w/ O_PATH. */
return fhandler_socket::link (newpath);
+ /* link on a socket file or linkat w/ AT_EMPTY_PATH called on a
+ socket opened w/ O_PATH. */
fhandler_disk_file fh (pc);
+ if (get_flags () & O_PATH)
+fh.set_handle (get_handle ());
return fh.link (newpath);
}
--
2.30.0
[PATCH 5/7] Cygwin: fix facl on sockets that are not socket files
If facl(2) is called on an AF_LOCAL or AF_UNIX socket that is not a
socket file, the current code calls fhandler_disk_file::facl in most
cases. The latter expects to be operating on a disk file and uses the
socket's io_handle, which is not a file handle.
Fix this by calling fhandler_disk_file::facl only if the
fhandler_socket object is a file (determined by testing dev().isfs()).
---
winsup/cygwin/fhandler_socket_local.cc | 6 +-
winsup/cygwin/fhandler_socket_unix.cc | 8 +---
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_socket_local.cc
b/winsup/cygwin/fhandler_socket_local.cc
index 349ade897..22586c0dd 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -737,8 +737,12 @@ fhandler_socket_local::fchown (uid_t uid, gid_t gid)
int
fhandler_socket_local::facl (int cmd, int nentries, aclent_t *aclbufp)
{
- if (get_sun_path () && get_sun_path ()[0] == '\0')
+ if (!dev ().isfs ())
+/* facl called on a socket. */
return fhandler_socket_wsock::facl (cmd, nentries, aclbufp);
+
+ /* facl on a socket file. [We won't get here if facl is called on a
+ socket opened w/ O_PATH.] */
fhandler_disk_file fh (pc);
return fh.facl (cmd, nentries, aclbufp);
}
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index 573864b9f..fae07367d 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -2408,10 +2408,12 @@ fhandler_socket_unix::fchown (uid_t uid, gid_t gid)
int
fhandler_socket_unix::facl (int cmd, int nentries, aclent_t *aclbufp)
{
- if (sun_path ()
- && (sun_path ()->un_len <= (socklen_t) sizeof (sa_family_t)
- || sun_path ()->un.sun_path[0] == '\0'))
+ if (!dev ().isfs ())
+/* facl called on a socket. */
return fhandler_socket::facl (cmd, nentries, aclbufp);
+
+ /* facl on a socket file. [We won't get here if facl is called on a
+ socket opened w/ O_PATH.] */
fhandler_disk_file fh (pc);
return fh.facl (cmd, nentries, aclbufp);
}
--
2.30.0
[PATCH 4/7] Cygwin: fix fchown on sockets that are not socket files
If fchown(2) is called on an AF_LOCAL or AF_UNIX socket that is not a
socket file, the current code calls fhandler_disk_file::fchown in most
cases. The latter expects to be operating on a disk file and uses the
socket's io_handle, which is not a file handle.
Fix this by calling fhandler_disk_file::fchown only if the
fhandler_socket object is a file (determined by testing dev().isfs()).
---
winsup/cygwin/fhandler_socket_local.cc | 6 +-
winsup/cygwin/fhandler_socket_unix.cc | 8 +---
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_socket_local.cc
b/winsup/cygwin/fhandler_socket_local.cc
index d1faa079a..349ade897 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -724,8 +724,12 @@ fhandler_socket_local::fchmod (mode_t newmode)
int
fhandler_socket_local::fchown (uid_t uid, gid_t gid)
{
- if (get_sun_path () && get_sun_path ()[0] == '\0')
+ if (!dev ().isfs ())
+/* fchown called on a socket. */
return fhandler_socket_wsock::fchown (uid, gid);
+
+ /* chown/lchown on a socket file. [We won't get here if fchown is
+ called on a socket opened w/ O_PATH.] */
fhandler_disk_file fh (pc);
return fh.fchown (uid, gid);
}
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index e08e9bdd9..573864b9f 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -2395,10 +2395,12 @@ fhandler_socket_unix::fchmod (mode_t newmode)
int
fhandler_socket_unix::fchown (uid_t uid, gid_t gid)
{
- if (sun_path ()
- && (sun_path ()->un_len <= (socklen_t) sizeof (sa_family_t)
- || sun_path ()->un.sun_path[0] == '\0'))
+ if (!dev ().isfs ())
+/* fchown called on a socket. */
return fhandler_socket::fchown (uid, gid);
+
+ /* chown/lchown on a socket file. [We won't get here if fchown is
+ called on a socket opened w/ O_PATH.] */
fhandler_disk_file fh (pc);
return fh.fchown (uid, gid);
}
--
2.30.0
[PATCH 1/7] Cygwin: fix fstat on sockets that are not socket files
If fstat(2) is called on an AF_LOCAL or AF_UNIX socket that is not a
socket file, the current code calls fstat_fs. The latter expects to
be operating on a disk file and uses the socket's io_handle, which is
not a file handle.
Fix this by calling fstat_fs only if the fhandler_socket object is a
file (determined by testing dev().isfs()).
---
winsup/cygwin/fhandler_socket_local.cc | 9 +
winsup/cygwin/fhandler_socket_unix.cc | 11 +--
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/winsup/cygwin/fhandler_socket_local.cc
b/winsup/cygwin/fhandler_socket_local.cc
index 964f3e819..f8adf6c46 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -673,11 +673,12 @@ fhandler_socket_local::fcntl (int cmd, intptr_t arg)
int __reg2
fhandler_socket_local::fstat (struct stat *buf)
{
- int res;
-
- if (get_sun_path () && get_sun_path ()[0] == '\0')
+ if (!dev ().isfs ())
+/* fstat called on a socket. */
return fhandler_socket_wsock::fstat (buf);
- res = fhandler_base::fstat_fs (buf);
+
+ /* stat/lstat on a socket file or fstat on a socket opened w/ O_PATH. */
+ int res = fhandler_base::fstat_fs (buf);
if (!res)
{
buf->st_mode = (buf->st_mode & ~S_IFMT) | S_IFSOCK;
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index eedb0847e..8091fa820 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -2337,13 +2337,12 @@ fhandler_socket_unix::fcntl (int cmd, intptr_t arg)
int __reg2
fhandler_socket_unix::fstat (struct stat *buf)
{
- int ret = 0;
-
- if (sun_path ()
- && (sun_path ()->un_len <= (socklen_t) sizeof (sa_family_t)
- || sun_path ()->un.sun_path[0] == '\0'))
+ if (!dev ().isfs ())
+/* fstat called on a socket. */
return fhandler_socket::fstat (buf);
- ret = fhandler_base::fstat_fs (buf);
+
+ /* stat/lstat on a socket file or fstat on a socket opened w/ O_PATH. */
+ int ret = fhandler_base::fstat_fs (buf);
if (!ret)
{
buf->st_mode = (buf->st_mode & ~S_IFMT) | S_IFSOCK;
--
2.30.0
[PATCH 2/7] Cygwin: fix fstatvfs on sockets that are not socket files
If fstatvfs(2) is called on an AF_LOCAL or AF_UNIX socket that is not
a socket file, the current code calls fhandler_disk_file::fstatvfs in
most cases. The latter expects to be operating on a disk file and
uses the socket's io_handle, which is not a file handle.
Fix this by calling fhandler_disk_file::fstatvfs only if the
fhandler_socket object is a socket file (determined by testing
dev().isfs()).
---
winsup/cygwin/fhandler_socket_local.cc | 5 -
winsup/cygwin/fhandler_socket_unix.cc | 14 +++---
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_socket_local.cc
b/winsup/cygwin/fhandler_socket_local.cc
index f8adf6c46..5ca6d8550 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -690,8 +690,11 @@ fhandler_socket_local::fstat (struct stat *buf)
int __reg2
fhandler_socket_local::fstatvfs (struct statvfs *sfs)
{
- if (get_sun_path () && get_sun_path ()[0] == '\0')
+ if (!dev ().isfs ())
+/* fstatvfs called on a socket. */
return fhandler_socket_wsock::fstatvfs (sfs);
+
+ /* statvfs on a socket file or fstatvfs on a socket opened w/ O_PATH. */
if (get_flags () & O_PATH)
/* We already have a handle. */
{
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index 8091fa820..06db929ed 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -2354,10 +2354,18 @@ fhandler_socket_unix::fstat (struct stat *buf)
int __reg2
fhandler_socket_unix::fstatvfs (struct statvfs *sfs)
{
- if (sun_path ()
- && (sun_path ()->un_len <= (socklen_t) sizeof (sa_family_t)
- || sun_path ()->un.sun_path[0] == '\0'))
+ if (!dev ().isfs ())
+/* fstatvfs called on a socket. */
return fhandler_socket::fstatvfs (sfs);
+
+ /* statvfs on a socket file or fstatvfs on a socket opened w/ O_PATH. */
+ if (get_flags () & O_PATH)
+/* We already have a handle. */
+{
+ HANDLE h = get_handle ();
+ if (h)
+ return fstatvfs_by_handle (h, sfs);
+}
fhandler_disk_file fh (pc);
fh.get_device () = FH_FS;
return fh.fstatvfs (sfs);
--
2.30.0
[PATCH 3/7] Cygwin: fix fchmod on sockets that are not socket files
If fchmod(2) is called on an AF_LOCAL or AF_UNIX socket that is not a
socket file, the current code calls fhandler_disk_file::fchmod in most
cases. The latter expects to be operating on a disk file and uses the
socket's io_handle, which is not a file handle.
Fix this by calling fhandler_disk_file::fchmod only if the
fhandler_socket object is a file (determined by testing dev().isfs()).
---
winsup/cygwin/fhandler_socket_local.cc | 6 +-
winsup/cygwin/fhandler_socket_unix.cc | 8 +---
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_socket_local.cc
b/winsup/cygwin/fhandler_socket_local.cc
index 5ca6d8550..d1faa079a 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -710,8 +710,12 @@ fhandler_socket_local::fstatvfs (struct statvfs *sfs)
int
fhandler_socket_local::fchmod (mode_t newmode)
{
- if (get_sun_path () && get_sun_path ()[0] == '\0')
+ if (!dev ().isfs ())
+/* fchmod called on a socket. */
return fhandler_socket_wsock::fchmod (newmode);
+
+ /* chmod on a socket file. [We won't get here if fchmod is called
+ on a socket opened w/ O_PATH.] */
fhandler_disk_file fh (pc);
fh.get_device () = FH_FS;
return fh.fchmod (S_IFSOCK | adjust_socket_file_mode (newmode));
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index 06db929ed..e08e9bdd9 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -2374,10 +2374,12 @@ fhandler_socket_unix::fstatvfs (struct statvfs *sfs)
int
fhandler_socket_unix::fchmod (mode_t newmode)
{
- if (sun_path ()
- && (sun_path ()->un_len <= (socklen_t) sizeof (sa_family_t)
- || sun_path ()->un.sun_path[0] == '\0'))
+ if (!dev ().isfs ())
+/* fchmod called on a socket. */
return fhandler_socket::fchmod (newmode);
+
+ /* chmod on a socket file. [We won't get here if fchmod is called
+ on a socket opened w/ O_PATH.] */
fhandler_disk_file fh (pc);
fh.get_device () = FH_FS;
/* Kludge: Don't allow to remove read bit on socket files for
--
2.30.0
[PATCH 0/7] Fix some system calls on sockets
Several of the fhandler_socket_local and fhandler_socket_unix methods that support system calls are written as though they are operating on socket files unless the socket is an abstract socket. This patchset (except for the last patch) attempts to fix this by checking whether the fhandler is associated with a socket file. If not, we call an fhandler_socket_wsock or fhandler_socket method instead of an fhandler_disk_file method. The last patch is just a code simplification that arose while I was working on fhandler_socket_local::link. Ken Brown (7): Cygwin: fix fstat on sockets that are not socket files Cygwin: fix fstatvfs on sockets that are not socket files Cygwin: fix fchmod on sockets that are not socket files Cygwin: fix fchown on sockets that are not socket files Cygwin: fix facl on sockets that are not socket files Cygwin: fix linkat(2) on sockets that are not socket files Cygwin: simplify linkat with AT_EMPTY_PATH winsup/cygwin/fhandler_socket_local.cc | 39 +- winsup/cygwin/fhandler_socket_unix.cc | 56 -- winsup/cygwin/syscalls.cc | 24 +++ 3 files changed, 81 insertions(+), 38 deletions(-) -- 2.30.0
[PATCH 0/1] Fix facl on files opened with O_PATH
I'm not sure if this patch is right. Should facl fail on all commands or just on SETACL? If the command is GETACL, for example, should this fail like fgetxattr(2) or should it succeed like fstat(2)? Cygwin may be the only platform that supports both facl(2) and O_PATH, so I guess we're on our own here. Ken Brown (1): Cygwin: facl: fail with EBADF on files opened with O_PATH winsup/cygwin/sec_acl.cc | 5 + 1 file changed, 5 insertions(+) -- 2.30.0
[PATCH 1/1] Cygwin: facl: fail with EBADF on files opened with O_PATH
This is in the spirit of the Linux requirement that file operations
like fchmod(2), fchown(2), and fgetxattr(2) fail with EBADF on files
opened with O_PATH.
---
winsup/cygwin/sec_acl.cc | 5 +
1 file changed, 5 insertions(+)
diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc
index fe015728d..90969b639 100644
--- a/winsup/cygwin/sec_acl.cc
+++ b/winsup/cygwin/sec_acl.cc
@@ -1246,6 +1246,11 @@ facl32 (int fd, int cmd, int nentries, aclent_t *aclbufp)
syscall_printf ("-1 = facl (%d)", fd);
return -1;
}
+ if (cfd->get_flags () & O_PATH)
+{
+ set_errno (EBADF);
+ return -1;
+}
int res = cfd->facl (cmd, nentries, aclbufp);
syscall_printf ("%R = facl(%s) )", res, cfd->get_name ());
return res;
--
2.30.0
[PATCH] Cygwin: AF_UNIX: allow opening with the O_PATH flag
This was done for the fhandler_socket_local class in commits
3a2191653a, 141437d374, and 477121317d, but the fhandler_socket_unix
class was overlooked.
---
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_socket_unix.cc | 24
2 files changed, 25 insertions(+)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 21e1df172..ad90cf33d 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -,6 +,7 @@ class fhandler_socket_unix : public fhandler_socket
int getsockname (struct sockaddr *name, int *namelen);
int getpeername (struct sockaddr *name, int *namelen);
int shutdown (int how);
+ int open (int flags, mode_t mode = 0);
int close ();
int getpeereid (pid_t *pid, uid_t *euid, gid_t *egid);
ssize_t recvmsg (struct msghdr *msg, int flags);
diff --git a/winsup/cygwin/fhandler_socket_unix.cc
b/winsup/cygwin/fhandler_socket_unix.cc
index 9f7f86c47..eedb0847e 100644
--- a/winsup/cygwin/fhandler_socket_unix.cc
+++ b/winsup/cygwin/fhandler_socket_unix.cc
@@ -1208,6 +1208,11 @@ fhandler_socket_unix::~fhandler_socket_unix ()
int
fhandler_socket_unix::dup (fhandler_base *child, int flags)
{
+ if (get_flags () & O_PATH)
+/* We're viewing the socket as a disk file, but fhandler_base::dup
+ suffices here. */
+return fhandler_base::dup (child, flags);
+
if (fhandler_socket::dup (child, flags))
{
__seterrno ();
@@ -1801,9 +1806,23 @@ fhandler_socket_unix::shutdown (int how)
return 0;
}
+int
+fhandler_socket_unix::open (int flags, mode_t mode)
+{
+ /* We don't support opening sockets unless O_PATH is specified. */
+ if (flags & O_PATH)
+return open_fs (flags, mode);
+
+ set_errno (EOPNOTSUPP);
+ return 0;
+}
+
int
fhandler_socket_unix::close ()
{
+ if (get_flags () & O_PATH)
+return fhandler_base::close ();
+
HANDLE evt = InterlockedExchangePointer (_termination_evt, NULL);
HANDLE thr = InterlockedExchangePointer (_wait_thr, NULL);
if (thr)
@@ -2281,6 +2300,11 @@ fhandler_socket_unix::ioctl (unsigned int cmd, void *p)
int
fhandler_socket_unix::fcntl (int cmd, intptr_t arg)
{
+ if (get_flags () & O_PATH)
+/* We're viewing the socket as a disk file, but
+ fhandler_base::fcntl suffices here. */
+return fhandler_base::fcntl (cmd, arg);
+
int ret = -1;
switch (cmd)
--
2.30.0
[PATCH 3/3] Cygwin: FIFO: temporarily keep a conv_handle in syscalls.cc:open
When a FIFO is opened, syscalls.cc:open always calls fstat on the
newly-created fhandler_fifo. This results from a call to
device_access_denied.
To speed-up this fstat call, and therefore the open(2) call, use
PC_KEEP_HANDLE when the fhandler is created. The resulting
conv_handle is retained until after the fstat call if the fhandler is
a FIFO; otherwise, it is closed immediately.
---
winsup/cygwin/syscalls.cc | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 32a155a1c..460fe6801 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -1487,8 +1487,15 @@ open (const char *unix_path, int flags, ...)
opt |= PC_CTTY;
}
+ /* If we're opening a FIFO, we will call device_access_denied
+below. This leads to a call to fstat, which can use the
+path_conv handle. */
+ opt |= PC_KEEP_HANDLE;
if (!(fh = build_fh_name (unix_path, opt, stat_suffixes)))
__leave;/* errno already set */
+ opt &= ~PC_KEEP_HANDLE;
+ if (!fh->isfifo ())
+ fh->pc.close_conv_handle ();
if ((flags & O_NOFOLLOW) && fh->issymlink () && !(flags & O_PATH))
{
set_errno (ELOOP);
@@ -1555,9 +1562,18 @@ open (const char *unix_path, int flags, ...)
delete fh;
fh = fh_file;
}
- else if ((fh->is_fs_special () && fh->device_access_denied (flags))
- || !fh->open_with_arch (flags, mode & 0))
- __leave;/* errno already set */
+ else
+ {
+ if (fh->is_fs_special ())
+ {
+ if (fh->device_access_denied (flags))
+ __leave;/* errno already set */
+ else if (fh->isfifo ())
+ fh->pc.close_conv_handle ();
+ }
+ if (!fh->open_with_arch (flags, mode & 0))
+ __leave;/* errno already set */
+ }
/* Move O_TMPFILEs to the bin to avoid blocking the parent dir. */
if ((flags & O_TMPFILE) && !fh->pc.isremote ())
try_to_bin (fh->pc, fh->get_handle (), DELETE,
--
2.30.0
[PATCH 2/3] Cygwin: fstat_helper: always use handle in call to get_file_attribute
Previously, the call to get_file_attribute for FIFOs set the first
argument to NULL instead of the handle h returned by get_stat_handle,
thereby forcing the file to be opened for fetching the security
descriptor in get_file_sd(). This was done because h might have been
a pipe handle rather than a file handle, and its permissions would not
necessarily reflect those of the file.
That situation can no longer occur with the new fhandler_fifo::fstat
introduced in the previous commit.
---
winsup/cygwin/fhandler_disk_file.cc | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/winsup/cygwin/fhandler_disk_file.cc
b/winsup/cygwin/fhandler_disk_file.cc
index ef9171bbf..6170427b0 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -475,8 +475,7 @@ fhandler_base::fstat_helper (struct stat *buf)
else if (pc.issocket ())
buf->st_mode = S_IFSOCK;
- if (!get_file_attribute (is_fs_special () && !pc.issocket () ? NULL : h, pc,
- >st_mode, >st_uid, >st_gid))
+ if (!get_file_attribute (h, pc, >st_mode, >st_uid, >st_gid))
{
/* If read-only attribute is set, modify ntsec return value */
if (::has_attribute (attributes, FILE_ATTRIBUTE_READONLY)
--
2.30.0
[PATCH 0/3] Fix fstat on FIFOs, part 2
The first patch fixes a bug, in which fstat on FIFOs sometimes used pipe handles instead of file handles. The second and third patches should improve the efficiency of fstat and open on FIFOs. Ken Brown (3): Cygwin: define fhandler_fifo::fstat Cygwin: fstat_helper: always use handle in call to get_file_attribute Cygwin: FIFO: temporarily keep a conv_handle in syscalls.cc:open winsup/cygwin/fhandler.h| 1 + winsup/cygwin/fhandler_disk_file.cc | 3 +-- winsup/cygwin/fhandler_fifo.cc | 23 +++ winsup/cygwin/syscalls.cc | 22 +++--- 4 files changed, 44 insertions(+), 5 deletions(-) -- 2.30.0
[PATCH 1/3] Cygwin: define fhandler_fifo::fstat
Previously fstat on a FIFO would call fhandler_base::fstat.
The latter is not appropriate if fhandler_fifo::open has already been
called (and O_PATH is not set), for the following reason. If a FIFO
has been opened as a writer or duplexer, then it has an io_handle that
is a pipe handle rather than a file handle. fhandler_base::fstat will
use this handle and potentially return incorrect results. If the FIFO
has been opened as a reader, then it has no io_handle, and a call to
fhandler_base::fstat will lead to a call to fhandler_base::open.
Opening the fhandler a second time can change it in undesired ways;
for example, it can modify the flags and status_flags.
The new fhandler_fifo::fstat avoids these problems by creating an
fhandler_disk_file and calling its fstat method in case
fhandler_fifo::open has already been called and O_PATH is not set.
---
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_fifo.cc | 23 +++
2 files changed, 24 insertions(+)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index e457e2785..78d9c7984 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1510,6 +1510,7 @@ public:
ssize_t __reg3 raw_write (const void *ptr, size_t ulen);
void fixup_after_fork (HANDLE);
void fixup_after_exec ();
+ int __reg2 fstat (struct stat *buf);
int __reg2 fstatvfs (struct statvfs *buf);
select_record *select_read (select_stuff *);
select_record *select_write (select_stuff *);
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 8b67037cb..365f14053 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -1494,6 +1494,29 @@ errout:
len = (size_t) -1;
}
+int __reg2
+fhandler_fifo::fstat (struct stat *buf)
+{
+ if (reader || writer || duplexer)
+{
+ /* fhandler_fifo::open has been called, and O_PATH is not set.
+We don't want to call fhandler_base::fstat. In the writer
+and duplexer cases we have a handle, but it's a pipe handle
+rather than a file handle, so it's not suitable for stat. In
+the reader case we don't have a handle, but
+fhandler_base::fstat would call fhandler_base::open, which
+would modify the flags and status_flags. */
+ fhandler_disk_file fh (pc);
+ fh.get_device () = FH_FS;
+ int res = fh.fstat (buf);
+ buf->st_dev = buf->st_rdev = dev ();
+ buf->st_mode = dev ().mode ();
+ buf->st_size = 0;
+ return res;
+}
+ return fhandler_base::fstat (buf);
+}
+
int __reg2
fhandler_fifo::fstatvfs (struct statvfs *sfs)
{
--
2.30.0
[PATCH 1/1] Revert "Cygwin: fstat_helper: always use handle in call to get_file_attribute"
This reverts commit 76dca77f049271e2529c25de8a396e65dbce615d. That
commit was based on the incorrect assumption that get_stat_handle,
when called on a FIFO in fstat_helper, would always return a handle
that is safe to use for getting the file information.
That assumption is true in many cases but not all. For example, if
the call to fstat_helper arises from a call to fstat(2) on a FIFO that
has been opened for writing, then get_stat_handle will return a pipe
handle instead of a file handle.
---
winsup/cygwin/fhandler_disk_file.cc | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler_disk_file.cc
b/winsup/cygwin/fhandler_disk_file.cc
index 5e58688b7..ef9171bbf 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -394,13 +394,12 @@ fhandler_base::fstat_fs (struct stat *buf)
return res;
}
-/* Called by fstat_by_handle and fstat_by_name. */
int __reg2
fhandler_base::fstat_helper (struct stat *buf)
{
IO_STATUS_BLOCK st;
FILE_COMPRESSION_INFORMATION fci;
- HANDLE h = get_stat_handle (); /* Should always be pc.handle(). */
+ HANDLE h = get_stat_handle ();
PFILE_ALL_INFORMATION pfai = pc.fai ();
ULONG attributes = pc.file_attributes ();
@@ -476,8 +475,8 @@ fhandler_base::fstat_helper (struct stat *buf)
else if (pc.issocket ())
buf->st_mode = S_IFSOCK;
- if (!get_file_attribute (h, pc, >st_mode, >st_uid,
- >st_gid))
+ if (!get_file_attribute (is_fs_special () && !pc.issocket () ? NULL : h, pc,
+ >st_mode, >st_uid, >st_gid))
{
/* If read-only attribute is set, modify ntsec return value */
if (::has_attribute (attributes, FILE_ATTRIBUTE_READONLY)
--
2.30.0
[PATCH 0/1] Fix fstat on FIFOs, part 1
Commit 76dca77f04 had a careless blunder, so this patch reverts it. Nevertheless, fstat(2) can be made more efficient on FIFOs, and I'm working on a separate patchset to do this right. It's worth doing, because every call to open(2) on a FIFO leads to a call chain device_access_denied --> fhaccess --> fstat, and this is one of the cases where greater efficiency is possible. Ken Brown (1): Revert "Cygwin: fstat_helper: always use handle in call to get_file_attribute" winsup/cygwin/fhandler_disk_file.cc | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) -- 2.30.0
Re: [PATCH 0/1] Recognizing native Windows AF_UNIX sockets
On 1/30/2021 11:34 AM, Ken Brown via Cygwin-patches wrote: This patch attempts to fix the problem reported here: https://cygwin.com/pipermail/cygwin/2020-September/246362.html See also the followup here: https://cygwin.com/pipermail/cygwin/2021-January/247666.html The problem, briefly, is that on certain recent versions of Windows 10, including 2004 but not 1909, native Windows AF_UNIX sockets are represented by reparse points that Cygwin doesn't recognize. As a result, tools like 'ls' and 'rm' don't work. I will get access to a machine running 2004 so I can test my patch, but I'm posting it now in case someone else wants to test it before I can. To test it, compile and run the program native_unix_socket.c appended below, and then try to remove the file foo.sock that it creates. This should fail on W10 2004 without my patch, but it should succeed like this with the patch: I've just tested on W10 20H2. In Cygwin 3.1.7 I see the problem: $ ./native_unix_socket.exe getsockname works fam = 1, len = 11 offsetof clen = 9 strlen = 8 name = foo.sock $ ls -l foo.sock -rw-r- 1 Unknown+User Unknown+Group 0 2021-01-30 15:51 foo.sock $ rm foo.sock rm: remove write-protected regular empty file 'foo.sock'? y rm: cannot remove 'foo.sock': Permission denied After I apply the patch, all is well: $ ./native_unix_socket.exe getsockname works fam = 1, len = 11 offsetof clen = 9 strlen = 8 name = foo.sock $ ls -l foo.sock -rwxr-xr-x 1 kbrown None 0 2021-01-30 15:52 foo.sock* $ rm foo.sock $ ls -l foo.sock ls: cannot access 'foo.sock': No such file or directory Ken
[PATCH 1/1] Cygwin: recognize native Windows AF_UNIX sockets as reparse points
Allow check_reparse_point_target to recognize reparse points with reparse tag IO_REPARSE_TAG_AF_UNIX. These are used in recent versions of Windows 10 to represent AF_UNIX sockets. check_reparse_point_target now returns PATH_REP on files of this type, so that they are treated as known reparse points (but not as sockets). This allows tools like 'rm', 'ls', etc. to operate on these files. Addresses: https://cygwin.com/pipermail/cygwin/2020-September/246362.html https://cygwin.com/pipermail/cygwin/2021-January/247666.html --- winsup/cygwin/path.cc | 4 1 file changed, 4 insertions(+) diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index 6dc162806..9d2184d6a 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -2640,6 +2640,10 @@ check_reparse_point_target (HANDLE h, bool remote, PREPARSE_DATA_BUFFER rp, return PATH_REP | PATH_REP_NOAPI; #endif } + else if (rp->ReparseTag == IO_REPARSE_TAG_AF_UNIX) +/* Native Windows AF_UNIX socket; recognize this as a reparse + point but not as a socket. */ +return PATH_REP; return 0; } -- 2.30.0
[PATCH 0/1] Recognizing native Windows AF_UNIX sockets
This patch attempts to fix the problem reported here:
https://cygwin.com/pipermail/cygwin/2020-September/246362.html
See also the followup here:
https://cygwin.com/pipermail/cygwin/2021-January/247666.html
The problem, briefly, is that on certain recent versions of Windows
10, including 2004 but not 1909, native Windows AF_UNIX sockets are
represented by reparse points that Cygwin doesn't recognize. As a
result, tools like 'ls' and 'rm' don't work.
I will get access to a machine running 2004 so I can test my patch,
but I'm posting it now in case someone else wants to test it before I
can. To test it, compile and run the program native_unix_socket.c
appended below, and then try to remove the file foo.sock that it
creates. This should fail on W10 2004 without my patch, but it should
succeed like this with the patch:
$ gcc -o native_unix_socket native_unix_socket.c -lws2_32
$ ./native_unix_socket.exe
getsockname works
fam = 1, len = 11
offsetof clen = 9
strlen = 8
name = foo.sock
$ ls -l foo.sock
-rwxr-xr-x 1 kbrown None 0 2021-01-30 10:46 foo.sock*
$ rm foo.sock
$ ls -l foo.sock
ls: cannot access 'foo.sock': No such file or directory
$ cat native_unix_socket.c
/* https://cygwin.com/pipermail/cygwin/2020-September/246362.html
gcc -o native_unix_socket native_unix_socket.c -lws2_32
*/
#define WIN32_LEAN_AND_MEAN
#include
#include
#include
#include
#include
/* #include */
#define WIN32_LEAN_AND_MEAN
#define UNIX_PATH_MAX 108
typedef struct sockaddr_un
{
ADDRESS_FAMILY sun_family; /* AF_UNIX */
char sun_path[UNIX_PATH_MAX]; /* pathname */
} SOCKADDR_UN, *PSOCKADDR_UN;
int __cdecl main(int argc, char *argv[])
{
WSADATA wsadata;
struct sockaddr_un addr;
socklen_t len;
int z = AF_UNIX;
SOCKET s, s0;
if (WSAStartup(MAKEWORD(2,2), ) != 0) {
printf("STartup failed\n");
return 0;
}
s0 = socket(AF_UNIX, SOCK_STREAM, 0);
memset(, 0, sizeof(addr));
addr.sun_family = AF_UNIX;
//strcpy(addr.sun_path, argv[1]);
strcpy(addr.sun_path, "foo.sock");
z = bind(s0, (const struct sockaddr *) , strlen(addr.sun_path) +
sizeof (addr.sun_family));
if (z != 0) {
printf("bind failed %ld\n", WSAGetLastError());
}
len = sizeof(addr);
z = getsockname(s0, (struct sockaddr *), );
if (z != 0) {
printf("getsockname failed %ld\n", WSAGetLastError());
} else {
printf("getsockname works\n");
printf("fam = %d, len = %d\n", addr.sun_family, len);
int clen = len - offsetof(struct sockaddr_un, sun_path);
printf("offsetof clen = %d\n", clen);
printf("strlen = %zd\n", strlen(addr.sun_path));
printf("name = %s\n", addr.sun_path);
}
}
Ken Brown (1):
Cygwin: recognize native Windows AF_UNIX sockets as reparse points
winsup/cygwin/path.cc | 4
1 file changed, 4 insertions(+)
--
2.30.0
[PATCH 4/4] Cygwin: include/cygwin/limits.h: new header
The new header defines some Cygwin-specific limits, using private names. It is included by include/limits.h. For example, we now have #define __OPEN_MAX 3200 in include/cygwin/limits.h and #define OPEN_MAX __OPEN_MAX in include/limits.h. The purpose is to hide implementation details from users who view . --- winsup/cygwin/include/cygwin/limits.h | 65 ++ winsup/cygwin/include/limits.h| 80 +++ 2 files changed, 98 insertions(+), 47 deletions(-) create mode 100644 winsup/cygwin/include/cygwin/limits.h diff --git a/winsup/cygwin/include/cygwin/limits.h b/winsup/cygwin/include/cygwin/limits.h new file mode 100644 index 0..f005d5742 --- /dev/null +++ b/winsup/cygwin/include/cygwin/limits.h @@ -0,0 +1,65 @@ +/* cygwin/limits.h + +This file is part of Cygwin. + +This software is a copyrighted work licensed under the terms of the +Cygwin license. Please consult the file "CYGWIN_LICENSE" for +details. */ + +#ifndef _CYGWIN_LIMITS_H__ +#define _CYGWIN_LIMITS_H__ + +#define __AIO_LISTIO_MAX 32 +#define __AIO_MAX 8 +#define __AIO_PRIO_DELTA_MAX 0 + +/* 32000 is the safe value used for Windows processes when called from + Cygwin processes. */ +#define __ARG_MAX 32000 +#define __ATEXIT_MAX 32 +#define __CHILD_MAX 256 +#define __DELAYTIMER_MAX __INT_MAX__ +#define __HOST_NAME_MAX 255 +#define __IOV_MAX 1024 +#define __LOGIN_NAME_MAX 256 /* equal to UNLEN defined in w32api/lmcons.h */ +#define __MQ_OPEN_MAX 256 +#define __MQ_PRIO_MAX INT_MAX +#define __OPEN_MAX 3200/* value of the old OPEN_MAX_MAX */ +#define __PAGESIZE 65536 +#define __PTHREAD_DESTRUCTOR_ITERATIONS 4 + +/* Tls has 1088 items - and we don't want to use them all :] */ +#define __PTHREAD_KEYS_MAX 1024 +/* Actually the minimum stack size is somewhat of a split personality. + The size parameter in a CreateThread call is the size of the initially + commited stack size, which can be specified as low as 4K. However, the + default *reserved* stack size is 1 Meg, unless the .def file specifies + another STACKSIZE value. And even if you specify a stack size below 64K, + the allocation granularity is in the way. You can never squeeze multiple + threads in the same allocation granularity slot. Oh well. */ +#define __PTHREAD_STACK_MIN 65536 + +/* FIXME: We only support one realtime signal in 32 bit mode, but +_POSIX_RTSIG_MAX is 8. */ +#if __WORDSIZE == 64 +#define __RTSIG_MAX 33 +#else +#define __RTSIG_MAX 1 +#endif +#define __SEM_VALUE_MAX 1147483648 +#define __SIGQUEUE_MAX 32 +#define __STREAM_MAX 20 +#define __SYMLOOP_MAX 10 +#define __TIMER_MAX 32 +#define __TTY_NAME_MAX 32 +#define __FILESIZEBITS 64 +#define __LINK_MAX 1024 +#define __MAX_CANON 255 +#define __MAX_INPUT 255 +#define __NAME_MAX 255 + +/* Keep in sync with __PATHNAME_MAX__ in cygwin/config.h */ +#define __PATH_MAX 4096 +#define __PIPE_BUF 4096 + +#endif /* _CYGWIN_LIMITS_H__ */ diff --git a/winsup/cygwin/include/limits.h b/winsup/cygwin/include/limits.h index 497d45419..6bdc9b40b 100644 --- a/winsup/cygwin/include/limits.h +++ b/winsup/cygwin/include/limits.h @@ -10,6 +10,7 @@ details. */ #include #include +#include #ifndef _MACH_MACHLIMITS_H_ @@ -156,67 +157,66 @@ details. */ /* Maximum number of I/O operations in a single list I/O call supported by the implementation. */ -#define AIO_LISTIO_MAX 32 +#define AIO_LISTIO_MAX __AIO_LISTIO_MAX /* Maximum number of outstanding asynchronous I/O operations supported by the implementation. */ -#define AIO_MAX 8 +#define AIO_MAX __AIO_MAX /* The maximum amount by which a process can decrease its asynchronous I/O priority level from its own scheduling priority. Not yet implemented. */ -#define AIO_PRIO_DELTA_MAX 0 +#define AIO_PRIO_DELTA_MAX __AIO_PRIO_DELTA_MAX /* Maximum number of bytes in arguments and environment passed in an exec - call. 32000 is the safe value used for Windows processes when called - from Cygwin processes. */ + call. */ #undef ARG_MAX -#define ARG_MAX 32000 +#define ARG_MAX __ARG_MAX #if __XSI_VISIBLE || __POSIX_VISIBLE >= 200809 /* Maximum number of functions that may be registered with atexit(). */ #undef ATEXIT_MAX -#define ATEXIT_MAX 32 +#define ATEXIT_MAX __ATEXIT_MAX #endif /* Maximum number of simultaneous processes per real user ID. */ #undef CHILD_MAX -#define CHILD_MAX 256 +#define CHILD_MAX __CHILD_MAX /* Maximum number of timer expiration overruns. Not yet implemented. */ #undef DELAYTIMER_MAX -#define DELAYTIMER_MAX __INT_MAX__ +#define DELAYTIMER_MAX __DELAYTIMER_MAX /* Maximum length of a host name. */ #undef HOST_NAME_MAX -#define HOST_NAME_MAX 255 +#define HOST_NAME_MAX __HOST_NAME_MAX #if __XSI_VISIBLE /* Maximum number of iovcnt in a writev (an arbitrary number) */ #undef IOV_MAX -#define IOV_MAX 1024 +#define IOV_MAX __IOV_MAX #endif /* Maximum number of characters in a login name. */ #undef LOGIN_NAME_MAX -#define
[PATCH 3/4] Cygwin: remove the OPEN_MAX_MAX macro
Replace all occurrences of OPEN_MAX_MAX by OPEN_MAX, and define the
latter to be 3200, which was the value of the former. In view of the
recent change to getdtablesize, there is no longer a need to
distinguish between these two macros.
---
winsup/cygwin/dtable.cc| 8
winsup/cygwin/dtable.h | 2 --
winsup/cygwin/fcntl.cc | 2 +-
winsup/cygwin/include/limits.h | 7 +++
winsup/cygwin/resource.cc | 2 +-
winsup/cygwin/syscalls.cc | 8
winsup/cygwin/sysconf.cc | 2 +-
7 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 9f4210797..ad4b59211 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -74,10 +74,10 @@ dtable::extend (size_t howmuch, size_t min)
size_t new_size = size + howmuch;
fhandler_base **newfds;
- if (new_size <= OPEN_MAX_MAX)
+ if (new_size <= OPEN_MAX)
/* ok */;
- else if (size < OPEN_MAX_MAX && min < OPEN_MAX_MAX)
-new_size = OPEN_MAX_MAX;
+ else if (size < OPEN_MAX && min < OPEN_MAX)
+new_size = OPEN_MAX;
else
{
set_errno (EMFILE);
@@ -735,7 +735,7 @@ dtable::dup3 (int oldfd, int newfd, int flags)
set_errno (EBADF);
goto done;
}
- if (newfd >= OPEN_MAX_MAX || newfd < 0)
+ if (newfd >= OPEN_MAX || newfd < 0)
{
syscall_printf ("new fd out of bounds: %d", newfd);
set_errno (EBADF);
diff --git a/winsup/cygwin/dtable.h b/winsup/cygwin/dtable.h
index 0f745a75a..e1a8461b8 100644
--- a/winsup/cygwin/dtable.h
+++ b/winsup/cygwin/dtable.h
@@ -10,8 +10,6 @@ details. */
/* Initial and increment values for cygwin's fd table */
#define NOFILE_INCR32
-/* Maximum size we allow expanding to. */
-#define OPEN_MAX_MAX (100 * NOFILE_INCR)
#include "thread.h"
#include "sync.h"
diff --git a/winsup/cygwin/fcntl.cc b/winsup/cygwin/fcntl.cc
index 9ef7e521f..507ba61f7 100644
--- a/winsup/cygwin/fcntl.cc
+++ b/winsup/cygwin/fcntl.cc
@@ -57,7 +57,7 @@ fcntl64 (int fd, int cmd, ...)
{
case F_DUPFD:
case F_DUPFD_CLOEXEC:
- if (arg >= 0 && arg < OPEN_MAX_MAX)
+ if (arg >= 0 && arg < OPEN_MAX)
{
int flags = cmd == F_DUPFD_CLOEXEC ? O_CLOEXEC : 0;
res = cygheap->fdtab.dup3 (fd, cygheap_fdnew ((arg) - 1), flags);
diff --git a/winsup/cygwin/include/limits.h b/winsup/cygwin/include/limits.h
index 6a55578f3..497d45419 100644
--- a/winsup/cygwin/include/limits.h
+++ b/winsup/cygwin/include/limits.h
@@ -208,12 +208,11 @@ details. */
#undef MQ_PRIO_MAX
#define MQ_PRIO_MAX INT_MAX
-/* # of open files per process. Actually it can be more since Cygwin
- grows the dtable as necessary. We define a reasonable limit here
- which is returned by getdtablesize(), sysconf(_SC_OPEN_MAX) and
+/* # of open files per process. This limit is returned by
+ getdtablesize(), sysconf(_SC_OPEN_MAX), and
getrlimit(RLIMIT_NOFILE). */
#undef OPEN_MAX
-#define OPEN_MAX 256
+#define OPEN_MAX 3200
/* Size in bytes of a page. */
#undef PAGESIZE
diff --git a/winsup/cygwin/resource.cc b/winsup/cygwin/resource.cc
index ac56acf8c..9e9d2 100644
--- a/winsup/cygwin/resource.cc
+++ b/winsup/cygwin/resource.cc
@@ -182,7 +182,7 @@ getrlimit (int resource, struct rlimit *rlp)
__get_rlimit_stack (rlp);
break;
case RLIMIT_NOFILE:
- rlp->rlim_cur = rlp->rlim_max = OPEN_MAX_MAX;
+ rlp->rlim_cur = rlp->rlim_max = OPEN_MAX;
break;
case RLIMIT_CORE:
rlp->rlim_cur = cygheap->rlim_core;
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index d293ff2c0..52a020f07 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -143,7 +143,7 @@ extern "C" int
dup2 (int oldfd, int newfd)
{
int res;
- if (newfd >= OPEN_MAX_MAX || newfd < 0)
+ if (newfd >= OPEN_MAX || newfd < 0)
{
set_errno (EBADF);
res = -1;
@@ -164,7 +164,7 @@ extern "C" int
dup3 (int oldfd, int newfd, int flags)
{
int res;
- if (newfd >= OPEN_MAX_MAX)
+ if (newfd >= OPEN_MAX)
{
set_errno (EBADF);
res = -1;
@@ -2878,7 +2878,7 @@ setdtablesize (int size)
}
if (size <= (int) cygheap->fdtab.size
- || cygheap->fdtab.extend (size - cygheap->fdtab.size, OPEN_MAX_MAX))
+ || cygheap->fdtab.extend (size - cygheap->fdtab.size, OPEN_MAX))
return 0;
return -1;
@@ -2887,7 +2887,7 @@ setdtablesize (int size)
extern "C" int
getdtablesize ()
{
- return OPEN_MAX_MAX;
+ return OPEN_MAX;
}
extern "C" int
diff --git a/winsup/cygwin/sysconf.cc b/winsup/cygwin/sysconf.cc
index d5d82bb4a..70cdb0fbd 100644
--- a/winsup/cygwin/sysconf.cc
+++ b/winsup/cygwin/sysconf.cc
@@ -511,7 +511,7 @@ static struct
{cons, {c:CHILD_MAX}}, /* 1, _SC_CHILD_MAX */
{cons, {c:CLOCKS_PER_SEC}}, /* 2, _SC_CLK_TCK */
{cons, {c:NGROUPS_MAX}}, /* 3, _SC_NGROUPS_MAX */
- {cons,
[PATCH 2/4] Cygwin: sysconf, getrlimit: don't call getdtablesize
Now that getdtablesize always returns OPEN_MAX_MAX, we can simplify
sysconf(_SC_OPEN_MAX) and getrlimit(RLIMIT_NOFILE) to just use that
same constant instead of calling getdtablesize.
---
winsup/cygwin/resource.cc | 5 +
winsup/cygwin/sysconf.cc | 11 +--
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/winsup/cygwin/resource.cc b/winsup/cygwin/resource.cc
index 9e39d3a04..ac56acf8c 100644
--- a/winsup/cygwin/resource.cc
+++ b/winsup/cygwin/resource.cc
@@ -182,10 +182,7 @@ getrlimit (int resource, struct rlimit *rlp)
__get_rlimit_stack (rlp);
break;
case RLIMIT_NOFILE:
- rlp->rlim_cur = getdtablesize ();
- if (rlp->rlim_cur < OPEN_MAX)
- rlp->rlim_cur = OPEN_MAX;
- rlp->rlim_max = OPEN_MAX_MAX;
+ rlp->rlim_cur = rlp->rlim_max = OPEN_MAX_MAX;
break;
case RLIMIT_CORE:
rlp->rlim_cur = cygheap->rlim_core;
diff --git a/winsup/cygwin/sysconf.cc b/winsup/cygwin/sysconf.cc
index 001da96ad..d5d82bb4a 100644
--- a/winsup/cygwin/sysconf.cc
+++ b/winsup/cygwin/sysconf.cc
@@ -21,15 +21,6 @@ details. */
#include "cpuid.h"
#include "clock.h"
-static long
-get_open_max (int in)
-{
- long max = getdtablesize ();
- if (max < OPEN_MAX)
-max = OPEN_MAX;
- return max;
-}
-
static long
get_page_size (int in)
{
@@ -520,7 +511,7 @@ static struct
{cons, {c:CHILD_MAX}}, /* 1, _SC_CHILD_MAX */
{cons, {c:CLOCKS_PER_SEC}}, /* 2, _SC_CLK_TCK */
{cons, {c:NGROUPS_MAX}}, /* 3, _SC_NGROUPS_MAX */
- {func, {f:get_open_max}},/* 4, _SC_OPEN_MAX */
+ {cons, {c:OPEN_MAX_MAX}},/* 4, _SC_OPEN_MAX */
{cons, {c:_POSIX_JOB_CONTROL}}, /* 5, _SC_JOB_CONTROL */
{cons, {c:_POSIX_SAVED_IDS}},/* 6, _SC_SAVED_IDS */
{cons, {c:_POSIX_VERSION}}, /* 7, _SC_VERSION */
--
2.30.0
[PATCH 0/4] getdtablesize, OPEN_MAX, etc.
This patchset is an extension of the patch submitted here: https://cygwin.com/pipermail/cygwin-patches/2021q1/011060.html That patch is included as the first patch in this set. The change to OPEN_MAX still needs testing to see if it has too much impact on the performance of tcsh. I've make a first attempt to implement the suggestion of adding a new header. At this writing I'm not completely sure that I fully understand the purpose of that. My choice of which macros to define in it might need to be changed. Ken Brown (4): Cygwin: getdtablesize: always return OPEN_MAX_MAX Cygwin: sysconf, getrlimit: don't call getdtablesize Cygwin: remove the OPEN_MAX_MAX macro Cygwin: include/cygwin/limits.h: new header winsup/cygwin/dtable.cc | 8 +-- winsup/cygwin/dtable.h| 2 - winsup/cygwin/fcntl.cc| 2 +- winsup/cygwin/include/cygwin/limits.h | 65 winsup/cygwin/include/limits.h| 85 +++ winsup/cygwin/resource.cc | 5 +- winsup/cygwin/syscalls.cc | 8 +-- winsup/cygwin/sysconf.cc | 11 +--- 8 files changed, 111 insertions(+), 75 deletions(-) create mode 100644 winsup/cygwin/include/cygwin/limits.h -- 2.30.0
[PATCH 1/4] Cygwin: getdtablesize: always return OPEN_MAX_MAX
According to the Linux man page for getdtablesize(3), the latter is
supposed to return "the maximum number of files a process can have
open, one more than the largest possible value for a file descriptor."
The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
now return that.
Previously getdtablesize returned the current size of cygheap->fdtab,
Cygwin's internal file descriptor table. But this is a dynamically
growing table, and its current size does not reflect an actual limit
on the number of open files.
With this change, gnulib now reports that getdtablesize and
fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
corresponding gnulib modules will no longer use gnulib replacements on
Cygwin.
---
winsup/cygwin/syscalls.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 82ddad46d..d293ff2c0 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2887,7 +2887,7 @@ setdtablesize (int size)
extern "C" int
getdtablesize ()
{
- return cygheap->fdtab.size;
+ return OPEN_MAX_MAX;
}
extern "C" int
--
2.30.0
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 5:28 PM, Ken Brown via Cygwin-patches wrote:
On 1/28/2021 11:13 AM, Corinna Vinschen via Cygwin-patches wrote:
On Jan 28 17:07, Corinna Vinschen via Cygwin-patches wrote:
On Jan 28 08:42, Ken Brown via Cygwin-patches wrote:
On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote:
On Jan 27 21:51, Ken Brown via Cygwin-patches wrote:
According to the Linux man page for getdtablesize(3), the latter is
supposed to return "the maximum number of files a process can have
open, one more than the largest possible value for a file descriptor."
The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
now return that.
Previously getdtablesize returned the current size of cygheap->fdtab,
Cygwin's internal file descriptor table. But this is a dynamically
growing table, and its current size does not reflect an actual limit
on the number of open files.
With this change, gnulib now reports that getdtablesize and
fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
corresponding gnulib modules will no longer use gnulib replacements on
Cygwin.
---
winsup/cygwin/syscalls.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 5da05b18a..1f16d54b9 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2887,7 +2887,7 @@ setdtablesize (int size)
extern "C" int
getdtablesize ()
{
- return cygheap->fdtab.size;
+ return OPEN_MAX_MAX;
}
getdtablesize is used internally, too. After this change, the values
returned by sysconf and getrlimit should be revisited as well.
They will now return OPEN_MAX_MAX, as I think they should. The only
question in my mind is whether to simplify the code by removing the calls to
getdtablesize, something like this (untested):
But then again, what happens with OPEN_MAX in limits.h? Linux removed
it entirely. Given we have such a limit and it's not flexible as on
Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define
OPEN_MAX as 3200?
...ideally by adding a file include/cygwin/limits.h included by
include/limits.h, which defines __OPEN_MAX et al, as required.
I'm not completely sure I follow. Do you mean include/cygwin/limits.h should
contain
#define __OPEN_MAX 3200
and include/limits.h should contain
#define OPEN_MAX __OPEN_MAX ?
For the sake of my education, could you explain the reason for this?
Trying to answer my own question, I guess the idea is to hide implementation
details from viewers of limits.h. Is that right? I took a stab at this and am
about to send a patchset. I'm not sure whether I made a reasonable choice of
"et al" in "__OPEN_MAX et al".
Ken
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 11:13 AM, Corinna Vinschen via Cygwin-patches wrote:
On Jan 28 17:07, Corinna Vinschen via Cygwin-patches wrote:
On Jan 28 08:42, Ken Brown via Cygwin-patches wrote:
On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote:
On Jan 27 21:51, Ken Brown via Cygwin-patches wrote:
According to the Linux man page for getdtablesize(3), the latter is
supposed to return "the maximum number of files a process can have
open, one more than the largest possible value for a file descriptor."
The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
now return that.
Previously getdtablesize returned the current size of cygheap->fdtab,
Cygwin's internal file descriptor table. But this is a dynamically
growing table, and its current size does not reflect an actual limit
on the number of open files.
With this change, gnulib now reports that getdtablesize and
fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
corresponding gnulib modules will no longer use gnulib replacements on
Cygwin.
---
winsup/cygwin/syscalls.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 5da05b18a..1f16d54b9 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2887,7 +2887,7 @@ setdtablesize (int size)
extern "C" int
getdtablesize ()
{
- return cygheap->fdtab.size;
+ return OPEN_MAX_MAX;
}
getdtablesize is used internally, too. After this change, the values
returned by sysconf and getrlimit should be revisited as well.
They will now return OPEN_MAX_MAX, as I think they should. The only
question in my mind is whether to simplify the code by removing the calls to
getdtablesize, something like this (untested):
But then again, what happens with OPEN_MAX in limits.h? Linux removed
it entirely. Given we have such a limit and it's not flexible as on
Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define
OPEN_MAX as 3200?
...ideally by adding a file include/cygwin/limits.h included by
include/limits.h, which defines __OPEN_MAX et al, as required.
I'm not completely sure I follow. Do you mean include/cygwin/limits.h should
contain
#define __OPEN_MAX 3200
and include/limits.h should contain
#define OPEN_MAX __OPEN_MAX ?
For the sake of my education, could you explain the reason for this?
Thanks.
Ken
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 11:07 AM, Corinna Vinschen via Cygwin-patches wrote:
On Jan 28 08:42, Ken Brown via Cygwin-patches wrote:
On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote:
On Jan 27 21:51, Ken Brown via Cygwin-patches wrote:
According to the Linux man page for getdtablesize(3), the latter is
supposed to return "the maximum number of files a process can have
open, one more than the largest possible value for a file descriptor."
The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
now return that.
Previously getdtablesize returned the current size of cygheap->fdtab,
Cygwin's internal file descriptor table. But this is a dynamically
growing table, and its current size does not reflect an actual limit
on the number of open files.
With this change, gnulib now reports that getdtablesize and
fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
corresponding gnulib modules will no longer use gnulib replacements on
Cygwin.
---
winsup/cygwin/syscalls.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 5da05b18a..1f16d54b9 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2887,7 +2887,7 @@ setdtablesize (int size)
extern "C" int
getdtablesize ()
{
- return cygheap->fdtab.size;
+ return OPEN_MAX_MAX;
}
getdtablesize is used internally, too. After this change, the values
returned by sysconf and getrlimit should be revisited as well.
They will now return OPEN_MAX_MAX, as I think they should. The only
question in my mind is whether to simplify the code by removing the calls to
getdtablesize, something like this (untested):
But then again, what happens with OPEN_MAX in limits.h? Linux removed
it entirely. Given we have such a limit and it's not flexible as on
Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define
OPEN_MAX as 3200?
Makes sense to me.
One problem is that there are some applications in the wild which run
loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open
descriptors. tcsh is one of them. It may slow done tcsh quite a bit
if the loop runs to 3200 now every time.
I don't use tcsh. Is it easy to test this?
Ken
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote:
On Jan 27 21:51, Ken Brown via Cygwin-patches wrote:
According to the Linux man page for getdtablesize(3), the latter is
supposed to return "the maximum number of files a process can have
open, one more than the largest possible value for a file descriptor."
The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
now return that.
Previously getdtablesize returned the current size of cygheap->fdtab,
Cygwin's internal file descriptor table. But this is a dynamically
growing table, and its current size does not reflect an actual limit
on the number of open files.
With this change, gnulib now reports that getdtablesize and
fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
corresponding gnulib modules will no longer use gnulib replacements on
Cygwin.
---
winsup/cygwin/syscalls.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 5da05b18a..1f16d54b9 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2887,7 +2887,7 @@ setdtablesize (int size)
extern "C" int
getdtablesize ()
{
- return cygheap->fdtab.size;
+ return OPEN_MAX_MAX;
}
getdtablesize is used internally, too. After this change, the values
returned by sysconf and getrlimit should be revisited as well.
They will now return OPEN_MAX_MAX, as I think they should. The only question in
my mind is whether to simplify the code by removing the calls to getdtablesize,
something like this (untested):
diff --git a/winsup/cygwin/resource.cc b/winsup/cygwin/resource.cc
index 9e39d3a04..ac56acf8c 100644
--- a/winsup/cygwin/resource.cc
+++ b/winsup/cygwin/resource.cc
@@ -182,10 +182,7 @@ getrlimit (int resource, struct rlimit *rlp)
__get_rlimit_stack (rlp);
break;
case RLIMIT_NOFILE:
- rlp->rlim_cur = getdtablesize ();
- if (rlp->rlim_cur < OPEN_MAX)
- rlp->rlim_cur = OPEN_MAX;
- rlp->rlim_max = OPEN_MAX_MAX;
+ rlp->rlim_cur = rlp->rlim_max = OPEN_MAX_MAX;
break;
case RLIMIT_CORE:
rlp->rlim_cur = cygheap->rlim_core;
diff --git a/winsup/cygwin/sysconf.cc b/winsup/cygwin/sysconf.cc
index 001da96ad..d5d82bb4a 100644
--- a/winsup/cygwin/sysconf.cc
+++ b/winsup/cygwin/sysconf.cc
@@ -21,15 +21,6 @@ details. */
#include "cpuid.h"
#include "clock.h"
-static long
-get_open_max (int in)
-{
- long max = getdtablesize ();
- if (max < OPEN_MAX)
-max = OPEN_MAX;
- return max;
-}
-
static long
get_page_size (int in)
{
@@ -520,7 +511,7 @@ static struct
{cons, {c:CHILD_MAX}}, /* 1, _SC_CHILD_MAX */
{cons, {c:CLOCKS_PER_SEC}}, /* 2, _SC_CLK_TCK */
{cons, {c:NGROUPS_MAX}}, /* 3, _SC_NGROUPS_MAX */
- {func, {f:get_open_max}},/* 4, _SC_OPEN_MAX */
+ {cons, {c:OPEN_MAX_MAX}},/* 4, _SC_OPEN_MAX */
{cons, {c:_POSIX_JOB_CONTROL}}, /* 5, _SC_JOB_CONTROL */
{cons, {c:_POSIX_SAVED_IDS}},/* 6, _SC_SAVED_IDS */
{cons, {c:_POSIX_VERSION}}, /* 7, _SC_VERSION */
WDYT?
Ken
[PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
According to the Linux man page for getdtablesize(3), the latter is
supposed to return "the maximum number of files a process can have
open, one more than the largest possible value for a file descriptor."
The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
now return that.
Previously getdtablesize returned the current size of cygheap->fdtab,
Cygwin's internal file descriptor table. But this is a dynamically
growing table, and its current size does not reflect an actual limit
on the number of open files.
With this change, gnulib now reports that getdtablesize and
fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
corresponding gnulib modules will no longer use gnulib replacements on
Cygwin.
---
winsup/cygwin/syscalls.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 5da05b18a..1f16d54b9 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -2887,7 +2887,7 @@ setdtablesize (int size)
extern "C" int
getdtablesize ()
{
- return cygheap->fdtab.size;
+ return OPEN_MAX_MAX;
}
extern "C" int
--
2.30.0
[PATCH v2] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
non-symlinks. Previously it always failed, as it does on Linux. But
POSIX permits it to succeed on non-symlinks even if it fails on
symlinks.
The reason for following POSIX rather than Linux is to make gnulib
report that fchmodat works on Cygwin. This improves the efficiency of
packages like GNU tar that use gnulib's fchmodat module. Previously
such packages would use a gnulib replacement for fchmodat on Cygwin.
---
winsup/cygwin/syscalls.cc | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 4cc8d07f5..5da05b18a 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4787,17 +4787,27 @@ fchmodat (int dirfd, const char *pathname, mode_t mode,
int flags)
tmp_pathbuf tp;
__try
{
- if (flags)
+ if (flags & ~AT_SYMLINK_NOFOLLOW)
{
- /* BSD has lchmod, but Linux does not. POSIX says
-AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks; but Linux
-blindly fails even for non-symlinks. */
- set_errno ((flags & ~AT_SYMLINK_NOFOLLOW) ? EINVAL : EOPNOTSUPP);
+ set_errno (EINVAL);
__leave;
}
char *path = tp.c_get ();
if (gen_full_path_at (path, dirfd, pathname))
__leave;
+ if (flags)
+ {
+ /* BSD has lchmod, but Linux does not. POSIX says
+AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks.
+Linux blindly fails even for non-symlinks, but we allow
+it to succeed. */
+ path_conv pc (path, PC_SYM_NOFOLLOW, stat_suffixes);
+ if (pc.issymlink ())
+ {
+ set_errno (EOPNOTSUPP);
+ __leave;
+ }
+ }
return chmod (path, mode);
}
__except (EFAULT) {}
--
2.30.0
Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
On 1/27/2021 8:27 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 27 08:22, Ken Brown via Cygwin-patches wrote: On 1/27/2021 7:40 AM, Corinna Vinschen via Cygwin-patches wrote: On Jan 26 16:30, Ken Brown via Cygwin-patches wrote: Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on non-symlinks. Previously it always failed, as it does on Linux. But POSIX permits it to succeed on non-symlinks even if it fails on symlinks. The reason for following POSIX rather than Linux is to make gnulib report that fchmodat works on Cygwin. This improves the efficiency of packages like GNU tar that use gnulib's fchmodat module. Previously such packages would use a gnulib replacement for fchmodat on Cygwin. Wait, what? So if Cygwin behaves like Linux, gnulib treats fchmodat as non-working? So what does gnulib do on a Linux system? Does it use its own fchmodat there, too? Apparently so. Here's a comment from gnulib's test program for fchmodat: /* Test whether fchmodat+AT_SYMLINK_NOFOLLOW works on non-symlinks. This test fails on GNU/Linux with glibc 2.31 (but not on GNU/kFreeBSD nor GNU/Hurd) and Cygwin 2.9. */ I agree that it's strange. ¯\_(ツ)_/¯ I'll go ahead and submit a revised patch for the record, and you can decide whether you want to deviate from Linux. My own opinion is that it can't be bad to support a flag that Linux doesn't support, but I don't feel strongly about it. BTW, the mistake in the first version of the patch is that I forgot to specify PC_SYM_NOFOLLOW in the path_conv constructor. Ken
