New FAQ entry about permissions since Cygwin 1.7.34
There have been several recent threads on the cygwin list stemming from the permissions change in 1.7.34. I've pointed people to the FAQ about ssh public key authentication, which is not the first place where someone with this problem is likely to look. The following patch attempts to remedy this: --- winsup/doc/ChangeLog | 4 winsup/doc/faq-using.xml | 42 ++ 2 files changed, 46 insertions(+) diff --git a/winsup/doc/ChangeLog b/winsup/doc/ChangeLog index 35935be..7e85a76 100644 --- a/winsup/doc/ChangeLog +++ b/winsup/doc/ChangeLog @@ -1,3 +1,7 @@ +2015-10-06 Ken Brown + + * faq-using.xml (faq.using.same-with-permissions): New entry. + 2015-09-07 Brian Inglis * faq-using.xml (faq.using.man): Replace makewhatis with mandb. diff --git a/winsup/doc/faq-using.xml b/winsup/doc/faq-using.xml index 7656880..0564504 100644 --- a/winsup/doc/faq-using.xml +++ b/winsup/doc/faq-using.xml @@ -1183,6 +1183,48 @@ Users group instead. + +Why do my files have extra permissions after updating to Cygwin 1.7.34? + + +The problem is exactly the same as with the key files of SSH. See +. + +The solution is the same: + + + $ ls -l * + -rw-rwxr--+ 1 user group 42 Nov 12 2010 file1 + -rw-rwxr--+ 1 user group 42 Nov 12 2010 file2 + $ setfacl -b * + $ ls -l * + -rw-r--r-- 1 user group 42 Nov 12 2010 file1 + -rw-r--r-- 1 user group 42 Nov 12 2010 file2 + + +You may find that newly-created files also have unexpected +permissions: + + + $ touch foo + $ ls -l foo + -rw-rwxr--+ 1 user group 42 Nov 12 2010 foo + + +This probably means that the directory in which you're creating +the files has unwanted default ACL entries that are inherited by +newly-created files and subdirectories. The solution is again the +same: + + + $ setfacl -b . + $ touch bar + $ ls -l bar + -rw-r--r-- 1 user group 42 Nov 12 2010 bar + + + + Why do my Tk programs not work anymore? -- 2.5.3 Ken
Re: New FAQ entry about permissions since Cygwin 1.7.34
On 10/20/2015 3:25 PM, Corinna Vinschen wrote: Hi Ken, On Oct 6 16:41, Ken Brown wrote: There have been several recent threads on the cygwin list stemming from the permissions change in 1.7.34. I've pointed people to the FAQ about ssh public key authentication, which is not the first place where someone with this problem is likely to look. The following patch attempts to remedy this: Unfortunately it doesn't apply cleanly. There are weird differences in whitespaces and a patch-breaking line wrap. Can you check and resend, please? Sorry, my mailer must have messed it up. Here it is as an attachment. Ken From d073a456341cb58d92ce75c2b5b46e1d2a54ec09 Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Tue, 6 Oct 2015 16:31:05 -0400 Subject: [PATCH] * winsup/doc/faq-using.xml (faq.using.same-with-permissions): New entry. --- winsup/doc/ChangeLog | 4 winsup/doc/faq-using.xml | 42 ++ 2 files changed, 46 insertions(+) diff --git a/winsup/doc/ChangeLog b/winsup/doc/ChangeLog index 35935be..7e85a76 100644 --- a/winsup/doc/ChangeLog +++ b/winsup/doc/ChangeLog @@ -1,3 +1,7 @@ +2015-10-06 Ken Brown + + * faq-using.xml (faq.using.same-with-permissions): New entry. + 2015-09-07 Brian Inglis * faq-using.xml (faq.using.man): Replace makewhatis with mandb. diff --git a/winsup/doc/faq-using.xml b/winsup/doc/faq-using.xml index 7656880..4cfc822 100644 --- a/winsup/doc/faq-using.xml +++ b/winsup/doc/faq-using.xml @@ -1183,6 +1183,48 @@ Users group instead. + +Why do my files have extra permissions after updating to Cygwin 1.7.34? + + +The problem is exactly the same as with the key files of SSH. See +. + +The solution is the same: + + + $ ls -l * + -rw-rwxr--+ 1 user group 42 Nov 12 2010 file1 + -rw-rwxr--+ 1 user group 42 Nov 12 2010 file2 + $ setfacl -b * + $ ls -l * + -rw-r--r-- 1 user group 42 Nov 12 2010 file1 + -rw-r--r-- 1 user group 42 Nov 12 2010 file2 + + +You may find that newly-created files also have unexpected +permissions: + + + $ touch foo + $ ls -l foo + -rw-rwxr--+ 1 user group 42 Nov 12 2010 foo + + +This probably means that the directory in which you're creating +the files has unwanted default ACL entries that are inherited by +newly-created files and subdirectories. The solution is again the +same: + + + $ setfacl -b . + $ touch bar + $ ls -l bar + -rw-r--r-- 1 user group 42 Nov 12 2010 bar + + + + Why do my Tk programs not work anymore? -- 2.5.3
Trivial fix to last change
cygwin1.dll doesn't build on x86 after the last commit (eed35ef). The
trivial patch attached fixes it.
Ken
From 1cd61c54994b2ba6c6ec1d1f8f1249f5f8fd4af3 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Fri, 11 Dec 2015 17:08:28 -0500
Subject: [PATCH] Fix regparm attribute of fhandler_base::fstat_helper
* winsup/cygwin/fhandler_disk_file.cc (fhandler_base::fstat_helper):
Align regparm attribute to declaration in fhandler.h.
---
winsup/cygwin/ChangeLog | 5 +
winsup/cygwin/fhandler_disk_file.cc | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 3c9804b..7079baa 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,8 @@
+2015-12-11 Ken Brown
+
+ * fhandler_disk_file.cc (fhandler_base::fstat_helper): Align
+ regparm attribute to declaration in fhandler.h.
+
2015-12-10 Corinna Vinschen
* path.h (class path_conv_handle): Use FILE_ALL_INFORMATION instead of
diff --git a/winsup/cygwin/fhandler_disk_file.cc
b/winsup/cygwin/fhandler_disk_file.cc
index fe9dd03..1dd1b8c 100644
--- a/winsup/cygwin/fhandler_disk_file.cc
+++ b/winsup/cygwin/fhandler_disk_file.cc
@@ -428,7 +428,7 @@ fhandler_base::fstat_fs (struct stat *buf)
return res;
}
-int __reg3
+int __reg2
fhandler_base::fstat_helper (struct stat *buf)
{
IO_STATUS_BLOCK st;
--
2.6.2
Fix 'make distclean'
Sometimes when a build of Cygwin fails, there will be a message
suggesting running 'make distclean'. But this fails to clean the
winsup/cygwin subdirectory, and the build still fails.
On the other hand, 'make clean' in winsup/cygwin removes two source
files, which have to be restored before one can rebuild.
The attached patch fixes both problems.
Ken
From 71c16ecc3126a41ae41cbf354428c64282952951 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sun, 5 Jun 2016 12:58:22 -0400
Subject: [PATCH] Allow 'make distclean' to clean winsup/cygwin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
But don’t let it remove source files.
---
winsup/Makefile.in| 4 ++--
winsup/cygwin/Makefile.in | 7 ---
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/winsup/Makefile.in b/winsup/Makefile.in
index 1fdf93a..9eca806 100644
--- a/winsup/Makefile.in
+++ b/winsup/Makefile.in
@@ -48,7 +48,7 @@ CLEAN_SUBDIRS=${patsubst %,clean_%,$(SUBDIRS)}
INSTALL_LICENSE:=@INSTALL_LICENSE@
-.PHONY: all install clean all-info info install-info install-license check \
+.PHONY: all install clean distclean all-info info install-info install-license
check \
$(SUBDIRS) $(INSTALL_SUBDIRS) $(CLEAN_SUBDIRS)
.SUFFIXES:
@@ -71,7 +71,7 @@ install-license: CYGWIN_LICENSE COPYING
install: Makefile $(INSTALL_LICENSE) $(INSTALL_SUBDIRS)
-clean: $(CLEAN_SUBDIRS)
+clean distclean: $(CLEAN_SUBDIRS)
all-info:
diff --git a/winsup/cygwin/Makefile.in b/winsup/cygwin/Makefile.in
index 43919bd..a548368 100644
--- a/winsup/cygwin/Makefile.in
+++ b/winsup/cygwin/Makefile.in
@@ -653,14 +653,15 @@ uninstall-man:
rm -f $(DESTDIR)$(mandir)/man7/`basename $$i` ; \
done
-clean:
- -rm -f *.o *.dll *.dbg *.a *.exp junk *.base version.cc *.exe *.d
*stamp* *_magic.h sigfe.s cygwin.def globals.h $(srcdir)/$(TLSOFFSETS_H)
$(srcdir)/devices.cc
+clean distclean realclean:
+ -rm -f *.o *.dll *.dbg *.a *.exp junk *.base version.cc *.exe *.d
*stamp* *_magic.h sigfe.s cygwin.def globals.h
-@$(MAKE) -C ${cygserver_blddir} libclean
-maintainer-clean realclean: clean
+maintainer-clean: clean
@echo "This command is intended for maintainers to use;"
@echo "it deletes files that may require special tools to rebuild."
-rm -fr configure
+ -rm -f $(srcdir)/$(TLSOFFSETS_H) $(srcdir)/devices.cc
# Rule to build LDSCRIPT
$(LDSCRIPT): $(LDSCRIPT).in
--
2.8.3
Declaration of crypt
According to Posix, including should bring in the declaration
of crypt. The glibc and FreeBSD headers are consistent with this, but
Cygwin's aren't.
$ cat test.c
#include
int
main (void)
{
const char *key = NULL;
const char *salt = NULL;
crypt (key, salt);
}
$ gcc -c test.c
test.c: In function ‘main’:
test.c:8:3: warning: implicit declaration of function ‘crypt’
[-Wimplicit-function-declaration]
crypt (key, salt);
^
The attached patch is one way to fix this. It means that cygwin-devel
would have to require libcrypt-devel.
I'm not sure if I used the right feature-test macro in the patch. It's
marked XSI by Posix, but using __XSI_VISIBLE didn't work.
Ken
P.S. Is cygwin-patches OK for this sort of thing, or should I have sent
it to the newlib list?
From 91ed7816e771a78170555db246e0e35dc6d2ca3e Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Wed, 8 Jun 2016 17:04:06 -0400
Subject: [PATCH] Make declare crypt
This is mandated by Posix and is done by the glibc and FreeBSD headers.
---
newlib/libc/include/sys/unistd.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index ef00575..ebae5d8 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -31,6 +31,9 @@ int _EXFUN(close, (int __fildes ));
#if __POSIX_VISIBLE >= 199209
size_t _EXFUN(confstr, (int __name, char *__buf, size_t __len));
#endif
+#if __BSD_VISIBLE && defined(__CYGWIN__)
+#include
+#endif
#if __XSI_VISIBLE && __XSI_VISIBLE < 700
char * _EXFUN(ctermid, (char *__s ));
#endif
--
2.8.3
Re: Declaration of crypt
On 6/9/2016 5:00 AM, Corinna Vinschen wrote:
Hi Ken,
On Jun 8 17:18, Ken Brown wrote:
According to Posix, including should bring in the declaration of
crypt. The glibc and FreeBSD headers are consistent with this, but Cygwin's
aren't.
$ cat test.c
#include
int
main (void)
{
const char *key = NULL;
const char *salt = NULL;
crypt (key, salt);
}
$ gcc -c test.c
test.c: In function ‘main’:
test.c:8:3: warning: implicit declaration of function ‘crypt’
[-Wimplicit-function-declaration]
crypt (key, salt);
^
The attached patch is one way to fix this. It means that cygwin-devel would
have to require libcrypt-devel.
I'm not sure if I used the right feature-test macro in the patch. It's
marked XSI by Posix, but using __XSI_VISIBLE didn't work.
What do you mean by "didn't work"? __XSI_VISIBLE should be the right
thing to use. Your application would have to define, e.g.,
_XOPEN_SOURCE before including the file.
Ah, that's what I missed. I tried defining __XSI_VISIBLE in the test
file, and I still got the implicit declaration warning. I see now,
reading /usr/include/sys/features.h, that __XSI_VISIBLE is a private
macro and shouldn't have been used in my test.
Another point is the && defined(__CYGWIN__). This should go away.
We're trying to make the headers more standards compatible without
going into too much detial what targat provides which function.
I wasn't sure that was portable to all newlib targets.
P.S. Is cygwin-patches OK for this sort of thing, or should I have sent it
to the newlib list?
Ideally to the newlib list, but no worries :)
OK, I'll do that next time.
Revised patch attached.
Ken
From cb8c2454618c1d862b926d61ac95411b384d0414 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Wed, 8 Jun 2016 17:04:06 -0400
Subject: [PATCH] Make declare crypt
This is mandated by Posix and is done by the glibc and FreeBSD headers.
---
newlib/libc/include/sys/unistd.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index ef00575..0cbb5de 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -31,6 +31,9 @@ int _EXFUN(close, (int __fildes ));
#if __POSIX_VISIBLE >= 199209
size_t _EXFUN(confstr, (int __name, char *__buf, size_t __len));
#endif
+#if __XSI_VISIBLE
+#include
+#endif
#if __XSI_VISIBLE && __XSI_VISIBLE < 700
char * _EXFUN(ctermid, (char *__s ));
#endif
--
2.8.3
Re: Declaration of crypt
On 6/9/2016 8:32 AM, Corinna Vinschen wrote: Can you please define crypt, encrypt and setkey explicitely in unistd.h per POSIX, rather than including crypt.h? This would not only be target independent, it would also be more correct. As a side effect I will have to come up with a new version of the crypt package, because our crypt.h is using a wrong prototypes for setkey (const is missing). setkey is supposed to be in stdlib.h rather than unistd.h, so I've done that. One minor question about encrypt: The Posix prototype has 'char block[64]' as the first argument, but Cygwin's crypt.h simply has 'char *block'. FreeBSD and glibc also use 'char *block', so I did the same. Or would you rather follow Posix here? Thanks a lot and sorry again, No problem. Revised patch attached. Ken From e6ab8bbf44d1042df3dd9e989967a649523d193d Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Wed, 8 Jun 2016 17:04:06 -0400 Subject: [PATCH] Declare crypt, encrypt, and setkey per Posix --- newlib/libc/include/stdlib.h | 3 +++ newlib/libc/include/sys/unistd.h | 6 ++ 2 files changed, 9 insertions(+) diff --git a/newlib/libc/include/stdlib.h b/newlib/libc/include/stdlib.h index 3194d6b..3d1b8a9 100644 --- a/newlib/libc/include/stdlib.h +++ b/newlib/libc/include/stdlib.h @@ -144,6 +144,9 @@ char * _EXFUN(realpath, (const char *__restrict path, char *__restrict resolved_ #if __BSD_VISIBLE int_EXFUN(rpmatch, (const char *response)); #endif +#if __XSI_VISIBLE +_VOID _EXFUN(setkey, (const char *__key)); +#endif _VOID _EXFUN(srand,(unsigned __seed)); double _EXFUN(strtod,(const char *__restrict __n, char **__restrict __end_PTR)); double _EXFUN(_strtod_r,(struct _reent *,const char *__restrict __n, char **__restrict __end_PTR)); diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h index ef00575..05b4f9d 100644 --- a/newlib/libc/include/sys/unistd.h +++ b/newlib/libc/include/sys/unistd.h @@ -31,6 +31,9 @@ int _EXFUN(close, (int __fildes )); #if __POSIX_VISIBLE >= 199209 size_t _EXFUN(confstr, (int __name, char *__buf, size_t __len)); #endif +#if __XSI_VISIBLE +char * _EXFUN(crypt, (const char *__key, const char *__salt)); +#endif #if __XSI_VISIBLE && __XSI_VISIBLE < 700 char * _EXFUN(ctermid, (char *__s )); #endif @@ -46,6 +49,9 @@ int _EXFUN(dup2, (int __fildes, int __fildes2 )); int _EXFUN(dup3, (int __fildes, int __fildes2, int flags)); int_EXFUN(eaccess, (const char *__path, int __mode)); #endif +#if __XSI_VISIBLE +void _EXFUN(encrypt, (char *__block, int __edflag)); +#endif #if __BSD_VISIBLE || (__XSI_VISIBLE && __XSI_VISIBLE < 500) void _EXFUN(endusershell, (void)); #endif -- 2.8.3
Documentation patch: Clarify Cygwin's support for Win32 paths
The attached patch is the one promised in https://www.cygwin.com/ml/cygwin/2016-08/msg00431.html Ken From e46d88d4141737f7f75ec1fb60b6e404f598a965 Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Tue, 23 Aug 2016 13:24:49 -0400 Subject: [PATCH] Clarify Cygwin's support for Win32 paths See https://www.cygwin.com/ml/cygwin/2016-08/msg00431.html. --- winsup/doc/pathnames.xml | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/winsup/doc/pathnames.xml b/winsup/doc/pathnames.xml index 3c0bdc1..6f9fefa 100644 --- a/winsup/doc/pathnames.xml +++ b/winsup/doc/pathnames.xml @@ -6,15 +6,18 @@ Introduction -Cygwin supports both POSIX- and Win32-style paths. Directory +The Cygwin DLL supports both POSIX- and Win32-style paths. Directory delimiters may be either forward slashes or backslashes. Paths using backslashes or starting with a drive letter are always handled as Win32 paths. POSIX paths must only use forward slashes as delimiter, otherwise they are treated as Win32 paths and file access might fail in surprising ways. -The usage of Win32 paths, though possible, is deprecated, -since it circumvents important internal path handling mechanisms. +Although the Cygwin DLL supports Win32 paths, not all +Cygwin applications support them. Moreover, the usage of Win32 paths +circumvents important internal path handling mechanisms. This usage +is therefore strongly deprecated and may be removed in a future +release of Cygwin. See and for more information. @@ -451,11 +454,12 @@ filesystem borders by commands like find -xdev. Using native Win32 paths -Using native Win32 paths in Cygwin, while possible, is generally +Using native Win32 paths in Cygwin, while often possible, is generally inadvisable. Those paths circumvent all internal integrity checking and bypass the information given in the Cygwin mount table. -The following paths are treated as native Win32 paths in Cygwin: +The following paths are treated as native Win32 paths by the +Cygwin DLL (but not necessarily by Cygwin applications): -- 2.8.3
Add _PC_CASE_INSENSITIVE flag to pathconf
Patch attached.
I tested it by running getconf.exe, and also as follows:
$ cat case_sens_test.c
#include
#include
void
test (const char *path)
{
int ret = pathconf (path, _PC_CASE_INSENSITIVE);
printf ("pathconf (\"%s\", _PC_CASE_INSENSITIVE) returns %d\n", path,
ret);
if (ret == -1)
perror (" pathconf");
}
int
main ()
{
test ("/tmp");
test ("/tmp/a");
test ("/cygdrive/c/cygwin");
test ("/");
test (".");
}
$ gcc case_sens_test.c
$ ./a
pathconf ("/tmp", _PC_CASE_INSENSITIVE) returns 0
pathconf ("/tmp/a", _PC_CASE_INSENSITIVE) returns -1
pathconf: No such file or directory
pathconf ("/cygdrive/c/cygwin", _PC_CASE_INSENSITIVE) returns 1
pathconf ("/", _PC_CASE_INSENSITIVE) returns 0
pathconf (".", _PC_CASE_INSENSITIVE) returns 0
This test was done, obviously, on a system with the obcaseinsensitive
registry key set to 0, and with /tmp/a non-existent. I also tested with
the registry key set to 1, with the expected results.
Ken
From 04b507edbfbbaade6ef19ba9302ac9a758886995 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 20 Oct 2016 15:35:54 -0400
Subject: [PATCH] Add _PC_CASE_INSENSITIVE to [f]pathconf
Update the getconf utility to support the new flag as well as
_PC_POSIX_PERMISSIONS and _PC_POSIX_SECURITY. These were previously
unsupported, probably as an oversight.
---
newlib/libc/include/sys/unistd.h | 1 +
winsup/cygwin/fhandler.cc | 2 ++
winsup/cygwin/include/cygwin/version.h | 3 ++-
winsup/cygwin/release/2.6.1| 2 ++
winsup/utils/getconf.c | 3 +++
5 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index 05b4f9d..143147d 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -490,6 +490,7 @@ int _EXFUN(unlinkat, (int, const char *, int));
#define _PC_POSIX_PERMISSIONS90
/* Ask for full POSIX permission support including uid/gid settings. */
#define _PC_POSIX_SECURITY 91
+#define _PC_CASE_INSENSITIVE 92
#endif
/*
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index d77ccd3..9ab52ad 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -1891,6 +1891,8 @@ fhandler_base::fpathconf (int v)
return pc.has_acls () || pc.fs_is_nfs ();
set_errno (EINVAL);
break;
+case _PC_CASE_INSENSITIVE:
+ return !!pc.objcaseinsensitive ();
default:
set_errno (EINVAL);
break;
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index 1c1402c..6ba602e 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -469,12 +469,13 @@ details. */
302: Export nl_langinfo_l.
303: Export pthread_getname_np, pthread_setname_np.
304: Export strerror_l, strptime_l, wcsftime_l.
+ 305: [f]pathconf flag _PC_CASE_INSENSITIVE added.
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 304
+#define CYGWIN_VERSION_API_MINOR 305
/* There is also a compatibity version number associated with the shared memory
regions. It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/release/2.6.1 b/winsup/cygwin/release/2.6.1
index 61ce2de..7ced3c4 100644
--- a/winsup/cygwin/release/2.6.1
+++ b/winsup/cygwin/release/2.6.1
@@ -1,6 +1,8 @@
What's new:
---
+- Add _PC_CASE_INSENSITIVE flag to [f]pathconf(3).
+
What changed:
-
diff --git a/winsup/utils/getconf.c b/winsup/utils/getconf.c
index 8539233..e6b319e 100644
--- a/winsup/utils/getconf.c
+++ b/winsup/utils/getconf.c
@@ -186,6 +186,7 @@ static const struct conf_variable conf_table[] =
{ "POSIX2_RE_DUP_MAX", CONSTANT, _POSIX2_RE_DUP_MAX
},
/* Variables from fpathconf() */
+ { "CASE_INSENSITIVE",PATHCONF,
_PC_CASE_INSENSITIVE},
{ "FILESIZEBITS",PATHCONF, _PC_FILESIZEBITS
},
{ "LINK_MAX",PATHCONF, _PC_LINK_MAX
},
{ "MAX_CANON", PATHCONF, _PC_MAX_CANON
},
@@ -195,10 +196,12 @@ static const struct conf_variable conf_table[] =
{ "PIPE_BUF",PATHCONF, _PC_PIPE_BUF
},
{ "POSIX2_SYMLINKS", PATHCONF, _PC_2_SYMLINKS
},
{ "POSIX_ALLOC_SIZE_MIN",PATHCONF, _PC_ALLOC_SIZE_MIN
},
+ { "POSIX_PERMISSIONS", PATHCONF, _PC_POSIX_PERMISSIONS
},
Define sigsetjmp/siglongjmp only if __POSIX_VISIBLE
The attached patch fixes the issue reported here:
https://cygwin.com/ml/cygwin/2017-08/msg00060.html
I'm not sure if I was correct in including RTEMS or if this should have
been Cygwin only. If the former, I probably should have sent this to
the Newlib list.
Ken
From 45060307b1e8e06a522cd95e9567155e98dda832 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Sun, 6 Aug 2017 17:40:43 -0400
Subject: [PATCH] Define sigsetjmp/siglongjmp only if __POSIX_VISIBLE
---
newlib/libc/include/machine/setjmp.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/newlib/libc/include/machine/setjmp.h
b/newlib/libc/include/machine/setjmp.h
index 2b4dd8b91..fc5639f3d 100644
--- a/newlib/libc/include/machine/setjmp.h
+++ b/newlib/libc/include/machine/setjmp.h
@@ -368,7 +368,7 @@ typedef int jmp_buf[_JBLEN];
_END_STD_C
-#if defined(__CYGWIN__) || defined(__rtems__)
+#if (defined(__CYGWIN__) || defined(__rtems__)) && __POSIX_VISIBLE
#include
#ifdef __cplusplus
@@ -376,6 +376,8 @@ extern "C" {
#endif
/* POSIX sigsetjmp/siglongjmp macros */
+
+
#ifdef _JBTYPE
typedef _JBTYPE sigjmp_buf[_JBLEN+1+((sizeof (_JBTYPE) + sizeof (sigset_t) - 1)
/sizeof (_JBTYPE))];
@@ -450,4 +452,4 @@ extern int _setjmp (jmp_buf);
#ifdef __cplusplus
}
#endif
-#endif /* __CYGWIN__ or __rtems__ */
+#endif /* (__CYGWIN__ or __rtems__) and __POSIX_VISIBLE */
--
2.13.2
Re: Define sigsetjmp/siglongjmp only if __POSIX_VISIBLE
On 8/6/2017 5:46 PM, Ken Brown wrote: The attached patch fixes the issue reported here: https://cygwin.com/ml/cygwin/2017-08/msg00060.html I'm not sure if I was correct in including RTEMS or if this should have been Cygwin only. If the former, I probably should have sent this to the Newlib list. Sorry, I accidentally made a whitespace change. Corrected patch attached. Ken From 50cceaf0e1d33ddf3fc62399bc15761af93fa29d Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Sun, 6 Aug 2017 17:40:43 -0400 Subject: [PATCH] Define sigsetjmp/siglongjmp only if __POSIX_VISIBLE --- newlib/libc/include/machine/setjmp.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/newlib/libc/include/machine/setjmp.h b/newlib/libc/include/machine/setjmp.h index 2b4dd8b91..e6c3142ed 100644 --- a/newlib/libc/include/machine/setjmp.h +++ b/newlib/libc/include/machine/setjmp.h @@ -368,7 +368,7 @@ typedef int jmp_buf[_JBLEN]; _END_STD_C -#if defined(__CYGWIN__) || defined(__rtems__) +#if (defined(__CYGWIN__) || defined(__rtems__)) && __POSIX_VISIBLE #include #ifdef __cplusplus @@ -450,4 +450,4 @@ extern int _setjmp (jmp_buf); #ifdef __cplusplus } #endif -#endif /* __CYGWIN__ or __rtems__ */ +#endif /* (__CYGWIN__ or __rtems__) and __POSIX_VISIBLE */ -- 2.13.2
renameat2
Linux has a system call 'renameat2' which is like renameat but has an extra
'flags' argument. In particular, one can pass the RENAME_NOREPLACE flag to
cause the rename to fail with EEXIST if the target of the rename exists. See
http://man7.org/linux/man-pages/man2/rename.2.html
macOS has a similar functionality, provided by the function 'renameatx_np' with
the flag RENAME_EXCL.
There's also a recently introduced Gnulib module 'renameat2', but it requires
two system calls on Cygwin (one to test existence and the second to do the
rename), so that there is a race condition. On Linux and macOS it uses
renameat2 and renameatx_np to avoid the race.
The attached patch implements renameat2 on Cygwin (but only supporting the
RENAME_NOREPLACE flag). I've written it so that a rename that just changes
case on a case-insensitive file system succeeds.
If the patch is accepted, I'll submit a second patch that documents the new
function.
Here's a simple test program:
$ cat rename_noreplace.c
#include
#include
#include
int
main ()
{
int res = renameat2 (AT_FDCWD, "foo", AT_FDCWD, "bar", RENAME_NOREPLACE);
if (res < 0)
perror ("renameat2");
}
$ gcc -o rename_noreplace rename_noreplace.c
$ touch foo bar
$ ./rename_noreplace.exe
renameat2: File exists
$ ls foo bar
bar foo
$ rm bar
$ ./rename_noreplace.exe
$ ls foo bar
ls: cannot access 'foo': No such file or directory
bar
Ken
From 23edb25c9c0b6a21aefb10372db945807b2b8e3f Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 17 Aug 2017 09:12:15 -0400
Subject: [PATCH] cygwin: Implement renameat2
Define the RENAME_* flags in as defined on Linux in
, but support only RENAME_NOREPLACE.
---
newlib/libc/include/stdio.h| 3 +++
winsup/cygwin/common.din | 1 +
winsup/cygwin/include/cygwin/fs.h | 5 +
winsup/cygwin/include/cygwin/version.h | 4 +++-
winsup/cygwin/syscalls.cc | 41 +-
5 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
index 5d8cb1092..331a1cf07 100644
--- a/newlib/libc/include/stdio.h
+++ b/newlib/libc/include/stdio.h
@@ -384,6 +384,9 @@ int _EXFUN(vdprintf, (int, const char *__restrict, __VALIST)
#endif
#if __ATFILE_VISIBLE
int_EXFUN(renameat, (int, const char *, int, const char *));
+# ifdef __CYGWIN__
+int_EXFUN(renameat2, (int, const char *, int, const char *, unsigned int));
+# endif
#endif
/*
diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 8da432b8a..ca6ff3cf9 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -1168,6 +1168,7 @@ remquof NOSIGFE
remquol NOSIGFE
rename SIGFE
renameat SIGFE
+renameat2 SIGFE
res_close = __res_close SIGFE
res_init = __res_init SIGFE
res_mkquery = __res_mkquery SIGFE
diff --git a/winsup/cygwin/include/cygwin/fs.h
b/winsup/cygwin/include/cygwin/fs.h
index f606ffc39..14454376f 100644
--- a/winsup/cygwin/include/cygwin/fs.h
+++ b/winsup/cygwin/include/cygwin/fs.h
@@ -19,4 +19,9 @@ details. */
#define BLKPBSZGET 0x127b
#define BLKGETSIZE64 0x00041268
+/* Flags for renameat2, from /usr/include/linux/fs.h. */
+#define RENAME_NOREPLACE (1 << 0)
+#define RENAME_EXCHANGE (1 << 1)
+#define RENAME_WHITEOUT (1 << 2)
+
#endif
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index efd4ac017..7640abfad 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -481,12 +481,14 @@ details. */
314: Export explicit_bzero.
315: Export pthread_mutex_timedlock.
316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock.
+ 317: Export renameat2. Add RENAME_NOREPLACE, RENAME_EXCHANGE,
+ RENAME_WHITEOUT.
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 316
+#define CYGWIN_VERSION_API_MINOR 317
/* 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/syscalls.cc b/winsup/cygwin/syscalls.cc
index 885931632..d756f5f35 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -60,6 +60,7 @@ details. */
#include "tls_pbuf.h"
#include "sync.h"
#include "child_info.h"
+#include /* needed for RENAME_NOREPLACE */
#undef _close
#undef _lseek
@@ -2048,8 +2049,12 @@ nt_path_has_executable_suffix (PUNICODE_STRING upath)
return false;
}
-extern "C" int
-rename (const char *oldpath, const char *newpath)
+/* If newpath names an existing file and the RENAME_NOREPLACE flag is
+ specified, fail with EEXIST. Exception: Don't fail if the pu
Re: renameat2
On 8/18/2017 10:33 AM, Eric Blake wrote:
Please only define RENAME_NOREPLACE for now; that way, software can
probe what is defined to know what will work (defining a flag that will
always be an error is not as useful as leaving it undefined - and while
we may add RENAME_EXCHANGE support, I don't see how we can ever do
RENAME_WHITEOUT).
Thanks for the feedback. Revised patch attached.
From 136b0dfd53e147002e134048658d20f452402c9f Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 17 Aug 2017 09:12:15 -0400
Subject: [PATCH] cygwin: Implement renameat2
Define the RENAME_NOREPLACE flag in as defined on Linux
in . The other RENAME_* flags defined on Linux are not
supported.
---
newlib/libc/include/stdio.h| 3 +++
winsup/cygwin/common.din | 1 +
winsup/cygwin/include/cygwin/fs.h | 6 +
winsup/cygwin/include/cygwin/version.h | 3 ++-
winsup/cygwin/syscalls.cc | 41 +-
5 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
index 5d8cb1092..331a1cf07 100644
--- a/newlib/libc/include/stdio.h
+++ b/newlib/libc/include/stdio.h
@@ -384,6 +384,9 @@ int _EXFUN(vdprintf, (int, const char *__restrict, __VALIST)
#endif
#if __ATFILE_VISIBLE
int_EXFUN(renameat, (int, const char *, int, const char *));
+# ifdef __CYGWIN__
+int_EXFUN(renameat2, (int, const char *, int, const char *, unsigned int));
+# endif
#endif
/*
diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 8da432b8a..ca6ff3cf9 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -1168,6 +1168,7 @@ remquof NOSIGFE
remquol NOSIGFE
rename SIGFE
renameat SIGFE
+renameat2 SIGFE
res_close = __res_close SIGFE
res_init = __res_init SIGFE
res_mkquery = __res_mkquery SIGFE
diff --git a/winsup/cygwin/include/cygwin/fs.h
b/winsup/cygwin/include/cygwin/fs.h
index f606ffc39..48b0cca45 100644
--- a/winsup/cygwin/include/cygwin/fs.h
+++ b/winsup/cygwin/include/cygwin/fs.h
@@ -19,4 +19,10 @@ details. */
#define BLKPBSZGET 0x127b
#define BLKGETSIZE64 0x00041268
+/* Flags for renameat2, from /usr/include/linux/fs.h. For now we
+ support only RENAME_NOREPLACE. */
+#define RENAME_NOREPLACE (1 << 0)
+/* #define RENAME_EXCHANGE (1 << 1) */
+/* #define RENAME_WHITEOUT (1 << 2) */
+
#endif
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index efd4ac017..0384f77da 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -481,12 +481,13 @@ details. */
314: Export explicit_bzero.
315: Export pthread_mutex_timedlock.
316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock.
+ 317: Export renameat2. Add RENAME_NOREPLACE.
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 316
+#define CYGWIN_VERSION_API_MINOR 317
/* 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/syscalls.cc b/winsup/cygwin/syscalls.cc
index 885931632..d756f5f35 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -60,6 +60,7 @@ details. */
#include "tls_pbuf.h"
#include "sync.h"
#include "child_info.h"
+#include /* needed for RENAME_NOREPLACE */
#undef _close
#undef _lseek
@@ -2048,8 +2049,12 @@ nt_path_has_executable_suffix (PUNICODE_STRING upath)
return false;
}
-extern "C" int
-rename (const char *oldpath, const char *newpath)
+/* If newpath names an existing file and the RENAME_NOREPLACE flag is
+ specified, fail with EEXIST. Exception: Don't fail if the purpose
+ of the rename is just to change the case of oldpath on a
+ case-insensitive file system. */
+static int
+rename2 (const char *oldpath, const char *newpath, unsigned int flags)
{
tmp_pathbuf tp;
int res = -1;
@@ -2068,6 +2073,12 @@ rename (const char *oldpath, const char *newpath)
__try
{
+ if (flags & ~RENAME_NOREPLACE)
+ /* RENAME_NOREPLACE is the only flag currently supported. */
+ {
+ set_errno (ENOTSUP);
+ __leave;
+ }
if (!*oldpath || !*newpath)
{
/* Reject rename("","x"), rename("x",""). */
@@ -2337,6 +2348,13 @@ rename (const char *oldpath, const char *newpath)
__leave;
}
+ /* Should we replace an existing file? */
+ if ((removepc || dstpc->exists ()) && (flags & RENAME_NOREPLACE))
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
+
/* Opening the file must be part of the transaction. It's not sufficient
to call only NtSetInformationF
Re: renameat2
Hi Corinna,
On 8/18/2017 11:15 AM, Corinna Vinschen wrote:
Hi Ken,
On Aug 18 09:21, Ken Brown wrote:
Linux has a system call 'renameat2' which is like renameat but has an
extra 'flags' argument. In particular, one can pass the
RENAME_NOREPLACE flag to cause the rename to fail with EEXIST if the
target of the rename exists. See
http://man7.org/linux/man-pages/man2/rename.2.html
macOS has a similar functionality, provided by the function
'renameatx_np' with the flag RENAME_EXCL.
There's also a recently introduced Gnulib module 'renameat2', but it
requires two system calls on Cygwin (one to test existence and the
second to do the rename), so that there is a race condition. On Linux
and macOS it uses renameat2 and renameatx_np to avoid the race.
The attached patch implements renameat2 on Cygwin (but only supporting
the RENAME_NOREPLACE flag). I've written it so that a rename that
just changes case on a case-insensitive file system succeeds.
If the patch is accepted, I'll submit a second patch that documents
the new function.
Neat stuff, but there are a few points for discussion, see below.
--- a/winsup/cygwin/include/cygwin/fs.h
+++ b/winsup/cygwin/include/cygwin/fs.h
@@ -19,4 +19,9 @@ details. */
#define BLKPBSZGET 0x127b
#define BLKGETSIZE64 0x00041268
+/* Flags for renameat2, from /usr/include/linux/fs.h. */
+#define RENAME_NOREPLACE (1 << 0)
+#define RENAME_EXCHANGE (1 << 1)
+#define RENAME_WHITEOUT (1 << 2)
Given that there's no standard for this call (yet), do we really want to
expose flag values we don't support? I would opt for only RENAME_NOREPLACE
for now and skip the others.
+
#endif
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index efd4ac017..7640abfad 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -481,12 +481,14 @@ details. */
314: Export explicit_bzero.
315: Export pthread_mutex_timedlock.
316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock.
+ 317: Export renameat2. Add RENAME_NOREPLACE, RENAME_EXCHANGE,
+ RENAME_WHITEOUT.
You can drop the flag values here. renameat2 is sufficient.
+rename2 (const char *oldpath, const char *newpath, unsigned int flags)
{
tmp_pathbuf tp;
int res = -1;
@@ -2068,6 +2073,12 @@ rename (const char *oldpath, const char *newpath)
__try
{
+ if (flags & ~RENAME_NOREPLACE)
+ /* RENAME_NOREPLACE is the only flag currently supported. */
+ {
+ set_errno (ENOTSUP);
That should ideally be EINVAL. Unsupported bit values in a flag argument?
EINVAL, please.
+ __leave;
+ }
if (!*oldpath || !*newpath)
{
/* Reject rename("","x"), rename("x",""). */
@@ -2337,6 +2348,13 @@ rename (const char *oldpath, const char *newpath)
__leave;
}
+ /* Should we replace an existing file? */
+ if ((removepc || dstpc->exists ()) && (flags & RENAME_NOREPLACE))
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
+
Do we really need this test here? If you check at this point and then
go ahead preparing the actual rename operation, you have the atomicity
problem again which renameat2 is trying to solve.
But there's light. NtSetInformationFile(FileRenameInformation) already
supports RENAME_NOREPLACE :)
Have a look at line 2494 (prior to your patch):
pfri->ReplaceIfExists = TRUE;
if you replace this with something like
pfri->ReplaceIfExists = !(flags & RENAME_NOREPLACE);
it should give us the atomic behaviour of renameat2 on Linux.
Another upside is, the status code returned is STATUS_OBJECT_NAME_COLLISION,
which translates to Win32 error ERROR_ALREADY_EXISTS, which in turn is
already converted to EEXIST by Cygwin, so there's nothing more to do :)
What do you think?
Thanks for the improvements! A revised patch is attached. As you'll
see, I still found a few places where I thought I needed to explicitly
set the errno to EEXIST. Let me know if any of these could be avoided.
Thanks.
Ken
From d5798b371ceabfe6a7912472edd32da1ebd7dcb7 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 17 Aug 2017 09:12:15 -0400
Subject: [PATCH] cygwin: Implement renameat2
Define the RENAME_NOREPLACE flag in as defined on Linux
in . The other RENAME_* flags defined on Linux are not
supported.
---
newlib/libc/include/stdio.h| 3 ++
winsup/cygwin/common.din | 1 +
winsup/cygwin/include/cygwin/fs.h | 6 +++
winsup/cygwin/include/cygwin/version.h | 3 +-
winsup/cygwin/syscalls.cc | 67 +++---
5 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
index 5d8cb1092..331a1c
Re: renameat2
Hi Corinna,
On 8/19/2017 5:57 AM, Corinna Vinschen wrote:
Hi Ken,
On Aug 18 18:24, Ken Brown wrote:
Thanks for the improvements! A revised patch is attached. As you'll see, I
still found a few places where I thought I needed to explicitly set the
errno to EEXIST. Let me know if any of these could be avoided.
No, you're right. Just one thing:
@@ -2410,6 +2433,11 @@ rename (const char *oldpath, const char *newpath)
unlink_nt returns with STATUS_DIRECTORY_NOT_EMPTY. */
if (dstpc->isdir ())
{
+ if (noreplace)
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
status = unlink_nt (*dstpc);
if (!NT_SUCCESS (status))
{
@@ -2423,6 +2451,11 @@ rename (const char *oldpath, const char *newpath)
due to a mangled suffix. */
else if (!removepc && dstpc->has_attribute (FILE_ATTRIBUTE_READONLY))
{
+ if (noreplace)
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
status = NtOpenFile (&nfh, FILE_WRITE_ATTRIBUTES,
dstpc->get_object_attr (attr, sec_none_nih),
&io, FILE_SHARE_VALID_FLAGS,
Both of the above cases are just border cases of one and the same
problem, the destination file already exists.
In retrospect your original patch was more concise:
+ /* Should we replace an existing file? */
+ if ((removepc || dstpc->exists ()) && (flags & RENAME_NOREPLACE))
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
The atomicity considerations are not affected by this test anyway, but
it would avoid starting and stopping a transaction on NTFS for no good
reason.
Maybe it's better to revert to this test and drop the other two again?
@@ -2491,11 +2524,15 @@ rename (const char *oldpath, const char *newpath)
__leave;
}
pfri = (PFILE_RENAME_INFORMATION) tp.w_get ();
- pfri->ReplaceIfExists = TRUE;
+ pfri->ReplaceIfExists = !noreplace;
pfri->RootDirectory = NULL;
pfri->FileNameLength = dstpc->get_nt_native_path ()->Length;
memcpy (&pfri->FileName, dstpc->get_nt_native_path ()->Buffer,
pfri->FileNameLength);
+ /* If dstpc points to an existing file and RENAME_NOREPLACE has
+been specified, then we should get NT error
+STATUS_OBJECT_NAME_COLLISION ==> Win32 error
+ERROR_ALREADY_EXISTS ==> Cygwin error EEXIST. */
status = NtSetInformationFile (fh, &io, pfri,
sizeof *pfri + pfri->FileNameLength,
FileRenameInformation);
@@ -2509,6 +2546,11 @@ rename (const char *oldpath, const char *newpath)
if (status == STATUS_ACCESS_DENIED && dstpc->exists ()
&& !dstpc->isdir ())
{
+ if (noremove)
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
Oh, right, that's a good catch.
The patch is ok as is, just let me know what you think of the above
minor tweak (and send the revised patch if you agree).
Yes, I agree. But can't I also drop the third test (where you said
"good catch") for the same reason? I've done that in the attached. If
I'm wrong and I still need that third test, let me know and I'll put it
back.
Thanks.
Ken
From bbff16b727172a459a9a052b0b948641c82d80eb Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 17 Aug 2017 09:12:15 -0400
Subject: [PATCH] cygwin: Implement renameat2
Define the RENAME_NOREPLACE flag in as defined on Linux
in . The other RENAME_* flags defined on Linux are not
supported.
---
newlib/libc/include/stdio.h| 3 +++
winsup/cygwin/common.din | 1 +
winsup/cygwin/include/cygwin/fs.h | 6 +
winsup/cygwin/include/cygwin/version.h | 3 ++-
winsup/cygwin/syscalls.cc | 48 +-
5 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
index 5d8cb1092..331a1cf07 100644
--- a/newlib/libc/include/stdio.h
+++ b/newlib/libc/include/stdio.h
@@ -384,6 +384,9 @@ int _EXFUN(vdprintf, (int, const char *__restrict, __VALIST)
#endif
#if __ATFILE_VISIBLE
int_EXFUN(renameat, (int, const char *, int, const char *));
+# ifdef __CYGWIN__
+int_EXFUN(renameat2, (int, const char *, int, const char *, unsigned int));
+# endif
#endif
/*
diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 8da432b8a..ca6ff3cf9 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -1168,6 +1168,7 @@ remquof NOSIGFE
remquol NOSIGFE
rename SIGFE
renameat SIGFE
+renameat2 SIGFE
res_close = __res_close SIGFE
res_init = __res_init SIGFE
res_mkquery = __res_mkquery SIGF
Re: renameat2
On 8/19/2017 12:37 PM, Corinna Vinschen wrote: > Oh, one more thing. This is a question to Yaakov, too. > > diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h > index 5d8cb1092..331a1cf07 100644 > --- a/newlib/libc/include/stdio.h > +++ b/newlib/libc/include/stdio.h > @@ -384,6 +384,9 @@ int _EXFUN(vdprintf, (int, const char *__restrict, > __VALIST) > #endif > #if __ATFILE_VISIBLE > int_EXFUN(renameat, (int, const char *, int, const char *)); > +# ifdef __CYGWIN__ > +int_EXFUN(renameat2, (int, const char *, int, const char *, unsigned > int)); > +# endif > #endif > > Does it makes sense to guard the renameat2 prototype more extensively > to cater for standards junkies? __MISC_VISIBLE, perhaps? I'll defer to Yaakov. But here's a related question. Is renameat currently guarded properly? The Linux man page says: Feature Test Macro Requirements for glibc (see feature_test_macros(7)): renameat(): Since glibc 2.10: _POSIX_C_SOURCE >= 200809L Before glibc 2.10: _ATFILE_SOURCE This suggests that we should do something like the following: diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h index 331a1cf07..9eb0964f2 100644 --- a/newlib/libc/include/stdio.h +++ b/newlib/libc/include/stdio.h @@ -381,8 +381,6 @@ FILE * _EXFUN(open_memstream, (char **, size_t *)); int_EXFUN(vdprintf, (int, const char *__restrict, __VALIST) _ATTRIBUTE ((__format__ (__printf__, 2, 0; # endif -#endif -#if __ATFILE_VISIBLE int_EXFUN(renameat, (int, const char *, int, const char *)); # ifdef __CYGWIN__ int_EXFUN(renameat2, (int, const char *, int, const char *, unsigned int)); Ken
Re: renameat2
On 8/19/2017 12:28 PM, Corinna Vinschen wrote: Doc changes coming? :) Attached. Ken From 0704541f1d29e0d9aa0af6e549f8ca0114a44a7c Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Sat, 19 Aug 2017 13:15:04 -0400 Subject: [PATCH] Document renameat2 --- winsup/cygwin/release/2.9.0 | 2 ++ winsup/doc/new-features.xml | 4 winsup/doc/posix.xml| 4 3 files changed, 10 insertions(+) diff --git a/winsup/cygwin/release/2.9.0 b/winsup/cygwin/release/2.9.0 index 421d6f24f..ac4c64949 100644 --- a/winsup/cygwin/release/2.9.0 +++ b/winsup/cygwin/release/2.9.0 @@ -6,6 +6,8 @@ What's new: - New APIs: pthread_mutex_timedwait, pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock. +- New API: renameat2. + What changed: - diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml index 23673d1e0..0aa857730 100644 --- a/winsup/doc/new-features.xml +++ b/winsup/doc/new-features.xml @@ -17,6 +17,10 @@ New APIs: pthread_mutex_timedwait, pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock. + +New API: renameat2. + + Improved implementation of <elf.h>. diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml index a2fffeebf..6e96272b7 100644 --- a/winsup/doc/posix.xml +++ b/winsup/doc/posix.xml @@ -1356,6 +1356,7 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008). ptsname_r putwc_unlocked putwchar_unlocked +renameat2 (see chapter "Implementation Notes") qsort_r(see chapter "Implementation Notes") quotactl rawmemchr @@ -1671,6 +1672,9 @@ group quotas, no inode quotas, no time constraints. qsort_r is available in both BSD and GNU flavors, depending on whether _BSD_SOURCE or _GNU_SOURCE is defined when compiling. +The Linux-specific function renameat2 only +supports the RENAME_NOREPLACE flag. + basename is available in both POSIX and GNU flavors, depending on whether libgen.h is included or not. -- 2.14.1
[PATCH 03/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::root
Fix all callers.
---
winsup/cygwin/fhandler_process.cc | 2 +-
winsup/cygwin/pinfo.cc| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/fhandler_process.cc
b/winsup/cygwin/fhandler_process.cc
index 97436dd1b..08cc7ea92 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -478,7 +478,7 @@ format_process_root (void *data, char *&destbuf)
cfree (destbuf);
destbuf = NULL;
}
- destbuf = p->root (fs);
+ destbuf = p ? p->root (fs) : NULL;
if (!destbuf || !*destbuf)
{
destbuf = cstrdup ("");
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index bba9ee494..9fe1b3a88 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -884,7 +884,7 @@ char *
_pinfo::root (size_t& n)
{
char *s;
- if (!this || !pid)
+ if (!pid)
return NULL;
if (pid != myself->pid && !ISSTATE (this, PID_NOTCYGWIN))
{
--
2.14.1
[PATCH 11/12] cygwin: Remove comparison of 'this' to NULL in _pinfo::exists
Fix all callers.
---
winsup/cygwin/fhandler_termios.cc | 2 +-
winsup/cygwin/pinfo.cc| 2 +-
winsup/cygwin/signal.cc | 2 +-
winsup/cygwin/sigproc.cc | 5 +++--
winsup/cygwin/times.cc| 10 +++---
5 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/winsup/cygwin/fhandler_termios.cc
b/winsup/cygwin/fhandler_termios.cc
index 19fcfc9cd..4ce53433a 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -131,7 +131,7 @@ tty_min::kill_pgrp (int sig)
for (unsigned i = 0; i < pids.npids; i++)
{
_pinfo *p = pids[i];
- if (!p->exists () || p->ctty != ntty || p->pgid != pgid)
+ if (!p || !p->exists () || p->ctty != ntty || p->pgid != pgid)
continue;
if (p == myself)
killself = sig != __SIGSETPGRP && !exit_state;
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 7193f6884..e4eef8b3c 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -529,7 +529,7 @@ _pinfo::set_ctty (fhandler_termios *fh, int flags)
bool __reg1
_pinfo::exists ()
{
- return this && process_state && !(process_state & (PID_EXITED | PID_REAPED |
PID_EXECED));
+ return process_state && !(process_state & (PID_EXITED | PID_REAPED |
PID_EXECED));
}
bool
diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
index 016fce1de..69c5e2aad 100644
--- a/winsup/cygwin/signal.cc
+++ b/winsup/cygwin/signal.cc
@@ -332,7 +332,7 @@ kill_pgrp (pid_t pid, siginfo_t& si)
{
_pinfo *p = pids[i];
- if (!p->exists ())
+ if (!p || !p->exists ())
continue;
/* Is it a process we want to kill? */
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 36fc64903..92fa5ea3d 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -152,7 +152,8 @@ proc_can_be_signalled (_pinfo *p)
bool __reg1
pid_exists (pid_t pid)
{
- return pinfo (pid)->exists ();
+ pinfo p (pid);
+ return p && p->exists ();
}
/* Return true if this is one of our children, false otherwise. */
@@ -1135,7 +1136,7 @@ remove_proc (int ci)
if (_my_tls._ctinfo != procs[ci].wait_thread)
procs[ci].wait_thread->terminate_thread ();
}
- else if (procs[ci]->exists ())
+ else if (procs[ci] && procs[ci]->exists ())
return true;
sigproc_printf ("removing procs[%d], pid %d, nprocs %d", ci, procs[ci]->pid,
diff --git a/winsup/cygwin/times.cc b/winsup/cygwin/times.cc
index fb480513f..5da0bbc7a 100644
--- a/winsup/cygwin/times.cc
+++ b/winsup/cygwin/times.cc
@@ -522,7 +522,7 @@ clock_gettime (clockid_t clk_id, struct timespec *tp)
pid = getpid ();
pinfo p (pid);
- if (!p->exists ())
+ if (!p || !p->exists ())
{
set_errno (EINVAL);
return -1;
@@ -746,8 +746,12 @@ clock_setres (clockid_t clk_id, struct timespec *tp)
extern "C" int
clock_getcpuclockid (pid_t pid, clockid_t *clk_id)
{
- if (pid != 0 && !pinfo (pid)->exists ())
-return (ESRCH);
+ if (pid != 0)
+{
+ pinfo p (pid);
+ if (!p || !p->exists ())
+ return (ESRCH);
+}
*clk_id = (clockid_t) PID_TO_CLOCKID (pid);
return 0;
}
--
2.14.1
[PATCH 02/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::fds
Fix all callers.
---
winsup/cygwin/fhandler_process.cc | 2 +-
winsup/cygwin/pinfo.cc| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/fhandler_process.cc
b/winsup/cygwin/fhandler_process.cc
index bbb44fa56..97436dd1b 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -375,7 +375,7 @@ format_process_fd (void *data, char *&destbuf)
{
if (destbuf)
cfree (destbuf);
- destbuf = p->fds (fs);
+ destbuf = p ? p->fds (fs) : NULL;
*((process_fd_t *) data)->fd_type = virt_symlink;
}
else
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index a504828fe..bba9ee494 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -856,7 +856,7 @@ char *
_pinfo::fds (size_t &n)
{
char *s;
- if (!this || !pid)
+ if (!pid)
return NULL;
if (pid != myself->pid)
{
--
2.14.1
[PATCH 01/12] cygwin: Remove comparisons of 'this' to 'NULL' in fhandler_dsp.cc
Fix all callers.
---
winsup/cygwin/fhandler_dsp.cc | 55 +++
1 file changed, 40 insertions(+), 15 deletions(-)
diff --git a/winsup/cygwin/fhandler_dsp.cc b/winsup/cygwin/fhandler_dsp.cc
index 5ae3309f8..b5c685bf8 100644
--- a/winsup/cygwin/fhandler_dsp.cc
+++ b/winsup/cygwin/fhandler_dsp.cc
@@ -63,7 +63,7 @@ class fhandler_dev_dsp::Audio
void convert_S16LE_S16BE (unsigned char *buffer, int size_bytes);
void fillFormat (WAVEFORMATEX * format,
int rate, int bits, int channels);
- unsigned blockSize (int rate, int bits, int channels);
+ static unsigned blockSize (int rate, int bits, int channels);
void (fhandler_dev_dsp::Audio::*convert_)
(unsigned char *buffer, int size_bytes);
@@ -115,6 +115,7 @@ class fhandler_dev_dsp::Audio_out: public Audio
void stop (bool immediately = false);
int write (const char *pSampleData, int nBytes);
void buf_info (audio_buf_info *p, int rate, int bits, int channels);
+ static void default_buf_info (audio_buf_info *p, int rate, int bits, int
channels);
void callback_sampledone (WAVEHDR *pHdr);
bool parsewav (const char *&pData, int &nBytes,
int rate, int bits, int channels);
@@ -149,6 +150,7 @@ public:
void stop ();
bool read (char *pSampleData, int &nBytes);
void buf_info (audio_buf_info *p, int rate, int bits, int channels);
+ static void default_buf_info (audio_buf_info *p, int rate, int bits, int
channels);
void callback_blockfull (WAVEHDR *pHdr);
private:
@@ -499,11 +501,11 @@ void
fhandler_dev_dsp::Audio_out::buf_info (audio_buf_info *p,
int rate, int bits, int channels)
{
- p->fragstotal = MAX_BLOCKS;
- if (this && dev_)
+ if (dev_)
{
/* If the device is running we use the internal values,
possibly set from the wave file. */
+ p->fragstotal = MAX_BLOCKS;
p->fragsize = blockSize (freq_, bits_, channels_);
p->fragments = Qisr2app_->query ();
if (pHdr_ != NULL)
@@ -514,10 +516,17 @@ fhandler_dev_dsp::Audio_out::buf_info (audio_buf_info *p,
}
else
{
+ default_buf_info(p, rate, bits, channels);
+}
+}
+
+void fhandler_dev_dsp::Audio_out::default_buf_info (audio_buf_info *p,
+int rate, int bits, int
channels)
+{
+ p->fragstotal = MAX_BLOCKS;
p->fragsize = blockSize (rate, bits, channels);
p->fragments = MAX_BLOCKS;
p->bytes = p->fragsize * p->fragments;
-}
}
/* This is called on an interupt so use locking.. Note Qisr2app_
@@ -951,14 +960,23 @@ fhandler_dev_dsp::Audio_in::waitfordata ()
return true;
}
+void fhandler_dev_dsp::Audio_in::default_buf_info (audio_buf_info *p,
+int rate, int bits, int
channels)
+{
+ p->fragstotal = MAX_BLOCKS;
+ p->fragsize = blockSize (rate, bits, channels);
+ p->fragments = 0;
+ p->bytes = 0;
+}
+
void
fhandler_dev_dsp::Audio_in::buf_info (audio_buf_info *p,
int rate, int bits, int channels)
{
- p->fragstotal = MAX_BLOCKS;
- p->fragsize = blockSize (rate, bits, channels);
- if (this && dev_)
+ if (dev_)
{
+ p->fragstotal = MAX_BLOCKS;
+ p->fragsize = blockSize (rate, bits, channels);
p->fragments = Qisr2app_->query ();
if (pHdr_ != NULL)
p->bytes = pHdr_->dwBytesRecorded - bufferIndex_
@@ -968,8 +986,7 @@ fhandler_dev_dsp::Audio_in::buf_info (audio_buf_info *p,
}
else
{
- p->fragments = 0;
- p->bytes = 0;
+ default_buf_info(p, rate, bits, channels);
}
}
@@ -1343,9 +1360,13 @@ fhandler_dev_dsp::_ioctl (unsigned int cmd, void *buf)
return -1;
}
audio_buf_info *p = (audio_buf_info *) buf;
- audio_out_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
- debug_printf ("buf=%p frags=%d fragsize=%d bytes=%d",
- buf, p->fragments, p->fragsize, p->bytes);
+if (audio_out_) {
+audio_out_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
+} else {
+Audio_out::default_buf_info(p, audiofreq_, audiobits_,
audiochannels_);
+}
+debug_printf ("buf=%p frags=%d fragsize=%d bytes=%d",
+ buf, p->fragments, p->fragsize, p->bytes);
return 0;
}
@@ -1357,9 +1378,13 @@ fhandler_dev_dsp::_ioctl (unsigned int cmd, void *buf)
return -1;
}
audio_buf_info *p = (audio_buf_info *) buf;
- audio_in_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
- debug_printf ("buf=%p frags=%d fragsize=%d bytes=%d",
- buf, p->fragments, p->fragsize, p->bytes);
+if (audio_in_) {
+audio_in_->buf_info (p, audiofreq_, audiobits_, audiochannels_);
+} else {
+Audio_in::default_buf_info(p, audiofreq_, audiobits_,
audiochannels_);
+}
+
[PATCH 12/12] cygwin: Remove workaround for GCC 6 null pointer check warnings
---
winsup/cygwin/Makefile.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/Makefile.in b/winsup/cygwin/Makefile.in
index 98727c019..add1de0e9 100644
--- a/winsup/cygwin/Makefile.in
+++ b/winsup/cygwin/Makefile.in
@@ -73,7 +73,7 @@ CRT0:=$(cygwin_build)/crt0.o
#
MT_SAFE:=@MT_SAFE@
CCEXTRA=
-COMMON_CFLAGS=-MMD ${$(*F)_CFLAGS} -Werror -fno-delete-null-pointer-checks
-fmerge-constants -ftracer $(CCEXTRA)
+COMMON_CFLAGS=-MMD ${$(*F)_CFLAGS} -Werror -fmerge-constants -ftracer
$(CCEXTRA)
ifeq ($(target_cpu),x86_64)
COMMON_CFLAGS+=-mcmodel=small
endif
--
2.14.1
[PATCH 06/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::commune_request
Fix all callers.
---
winsup/cygwin/pinfo.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index e792a0a1c..a068bcc42 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -720,7 +720,7 @@ _pinfo::commune_request (__uint32_t code, ...)
res.s = NULL;
res.n = 0;
- if (!this || !pid)
+ if (!pid)
{
set_errno (ESRCH);
goto err;
--
2.14.1
[PATCH 07/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::pipe_fhandler
Fix all callers.
---
winsup/cygwin/pinfo.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index a068bcc42..72aa658eb 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -819,7 +819,7 @@ out:
fhandler_pipe *
_pinfo::pipe_fhandler (int64_t unique_id, size_t &n)
{
- if (!this || !pid)
+ if (!pid)
return NULL;
if (pid == myself->pid)
return NULL;
--
2.14.1
[PATCH 08/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::fd
Fix all callers.
---
winsup/cygwin/fhandler_process.cc | 2 +-
winsup/cygwin/pinfo.cc| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/fhandler_process.cc
b/winsup/cygwin/fhandler_process.cc
index 7f122fbe4..360d8ea81 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -391,7 +391,7 @@ format_process_fd (void *data, char *&destbuf)
set_errno (ENOENT);
return 0;
}
- destbuf = p->fd (fd, fs);
+ destbuf = p ? p->fd (fd, fs) : NULL;
if (!destbuf || !*destbuf)
{
set_errno (ENOENT);
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 72aa658eb..044c4fc3c 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -832,7 +832,7 @@ char *
_pinfo::fd (int fd, size_t &n)
{
char *s;
- if (!this || !pid)
+ if (!pid)
return NULL;
if (pid != myself->pid)
{
--
2.14.1
[PATCH 10/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::kill
Fix all callers.
---
winsup/cygwin/signal.cc | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
index fbd2b241e..016fce1de 100644
--- a/winsup/cygwin/signal.cc
+++ b/winsup/cygwin/signal.cc
@@ -260,7 +260,7 @@ _pinfo::kill (siginfo_t& si)
}
this_pid = pid;
}
- else if (this && process_state == PID_EXITED)
+ else if (process_state == PID_EXITED)
{
this_process_state = process_state;
this_pid = pid;
@@ -296,8 +296,17 @@ kill0 (pid_t pid, siginfo_t& si)
syscall_printf ("signal %d out of range", si.si_signo);
return -1;
}
-
- return (pid > 0) ? pinfo (pid)->kill (si) : kill_pgrp (-pid, si);
+ if (pid > 0)
+{
+ pinfo p (pid);
+ if (!p)
+ {
+ set_errno (ESRCH);
+ return -1;
+ }
+ return p->kill (si);
+}
+ return kill_pgrp (-pid, si);
}
int
--
2.14.1
[PATCH 04/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::cwd
Fix all callers.
---
winsup/cygwin/fhandler_process.cc | 2 +-
winsup/cygwin/pinfo.cc| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/fhandler_process.cc
b/winsup/cygwin/fhandler_process.cc
index 08cc7ea92..453e79b16 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -498,7 +498,7 @@ format_process_cwd (void *data, char *&destbuf)
cfree (destbuf);
destbuf = NULL;
}
- destbuf = p->cwd (fs);
+ destbuf = p ? p->cwd (fs) : NULL;
if (!destbuf || !*destbuf)
{
destbuf = cstrdup ("");
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 9fe1b3a88..c28158168 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -930,7 +930,7 @@ char *
_pinfo::cwd (size_t& n)
{
char *s = NULL;
- if (!this || !pid)
+ if (!pid)
return NULL;
if (ISSTATE (this, PID_NOTCYGWIN))
{
--
2.14.1
[PATCH 09/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::environ
Fix all callers.
---
winsup/cygwin/fhandler_process.cc | 2 +-
winsup/cygwin/pinfo.cc| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/fhandler_process.cc
b/winsup/cygwin/fhandler_process.cc
index 360d8ea81..eb5a523ff 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -583,7 +583,7 @@ format_process_environ (void *data, char *&destbuf)
cfree (destbuf);
destbuf = NULL;
}
- destbuf = p->environ (fs);
+ destbuf = p ? p->environ (fs) : NULL;
if (!destbuf || !*destbuf)
{
destbuf = cstrdup ("");
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 044c4fc3c..7193f6884 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -1036,7 +1036,7 @@ char *
_pinfo::environ (size_t& n)
{
char **env = NULL;
- if (!this || !pid)
+ if (!pid)
return NULL;
if (ISSTATE (this, PID_NOTCYGWIN))
{
--
2.14.1
[PATCH 05/12] cygwin: Remove comparison of 'this' to 'NULL' in _pinfo::cmdline
Fix all callers.
---
winsup/cygwin/external.cc | 2 +-
winsup/cygwin/fhandler_process.cc | 2 +-
winsup/cygwin/pinfo.cc| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc
index c81bdc0fa..6aae32aea 100644
--- a/winsup/cygwin/external.cc
+++ b/winsup/cygwin/external.cc
@@ -339,7 +339,7 @@ cygwin_internal (cygwin_getinfo_types t, ...)
size_t n;
pid_t pid = va_arg (arg, pid_t);
pinfo p (pid);
- res = (uintptr_t) p->cmdline (n);
+ res = (uintptr_t) (p ? p->cmdline (n) : NULL);
}
break;
case CW_CHECK_NTSEC:
diff --git a/winsup/cygwin/fhandler_process.cc
b/winsup/cygwin/fhandler_process.cc
index 453e79b16..7f122fbe4 100644
--- a/winsup/cygwin/fhandler_process.cc
+++ b/winsup/cygwin/fhandler_process.cc
@@ -518,7 +518,7 @@ format_process_cmdline (void *data, char *&destbuf)
cfree (destbuf);
destbuf = NULL;
}
- destbuf = p->cmdline (fs);
+ destbuf = p ? p->cmdline (fs) : NULL;
if (!destbuf || !*destbuf)
{
destbuf = cstrdup ("");
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index c28158168..e792a0a1c 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -976,7 +976,7 @@ char *
_pinfo::cmdline (size_t& n)
{
char *s = NULL;
- if (!this || !pid)
+ if (!pid)
return NULL;
if (ISSTATE (this, PID_NOTCYGWIN))
{
--
2.14.1
[PATCH] cygcheck: Fix parsing of file names containing colons
Up to now the function winsup/utils/dump_setup.cc:base skips past
colons when parsing file names. As a result, a line like
foo foo-1:2.3-4.tar.bz2 1
in /etc/setup/installed.db would cause 'cygcheck -cd foo' to report 4
as the installed version of foo insted of 1:2.3-4. This is not an
issue now, but it will become an issue when version numbers are
allowed to contain epochs.
---
winsup/utils/dump_setup.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/dump_setup.cc
index 320d69fab..3922b18f8 100644
--- a/winsup/utils/dump_setup.cc
+++ b/winsup/utils/dump_setup.cc
@@ -56,7 +56,7 @@ base (const char *s)
const char *rv = s;
while (*s)
{
- if ((*s == '/' || *s == ':' || *s == '\\') && s[1])
+ if ((*s == '/' || *s == '\\') && s[1])
rv = s + 1;
s++;
}
--
2.14.2
Re: [PATCH] cygcheck: Fix parsing of file names containing colons
On 10/25/2017 8:19 AM, Corinna Vinschen wrote:
On Oct 25 14:11, Corinna Vinschen wrote:
Hi Ken,
On Oct 25 07:23, Ken Brown wrote:
Up to now the function winsup/utils/dump_setup.cc:base skips past
colons when parsing file names. As a result, a line like
foo foo-1:2.3-4.tar.bz2 1
in /etc/setup/installed.db would cause 'cygcheck -cd foo' to report 4
as the installed version of foo insted of 1:2.3-4. This is not an
issue now, but it will become an issue when version numbers are
allowed to contain epochs.
---
winsup/utils/dump_setup.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/dump_setup.cc
index 320d69fab..3922b18f8 100644
--- a/winsup/utils/dump_setup.cc
+++ b/winsup/utils/dump_setup.cc
@@ -56,7 +56,7 @@ base (const char *s)
const char *rv = s;
while (*s)
{
- if ((*s == '/' || *s == ':' || *s == '\\') && s[1])
+ if ((*s == '/' || *s == '\\') && s[1])
I think this is a simplified way to test for the colon in paths like
C:/foo/bar. Nothing else makes sense in this context.
I'm not sure how much we care, but maybe we shoulkd fix the test to
ignore the colon only if it's the second character in the incoming
string?
Not "ignore", but "use as a delimiter" only as 2nd char in the input.
I'm not sure the distinction matters in this case, since the function is
just trying to get the base name. Anyway, how's the attached?
Ken
From 1354e67e12819519f9a5440f12ef893e4e0bf7a5 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 24 Oct 2017 18:21:53 -0400
Subject: [PATCH] cygcheck: Fix parsing of file names containing colons
Up to now the function winsup/utils/dump_setup.cc:base skips past
colons when parsing file names. As a result, a line like
foo foo-1:2.3-4.tar.bz2 1
in /etc/setup/installed.db would cause 'cygcheck -cd foo' to report 4
as the installed version of foo insted of 1:2.3-4. This is not an
issue now, but it will become an issue when version numbers are
allowed to contain epochs.
---
winsup/utils/dump_setup.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/dump_setup.cc
index 320d69fab..d05817239 100644
--- a/winsup/utils/dump_setup.cc
+++ b/winsup/utils/dump_setup.cc
@@ -53,10 +53,12 @@ base (const char *s)
{
if (!s)
return 0;
+ if (isalpha (*s) && s[1] == ':')
+s += 2;
const char *rv = s;
while (*s)
{
- if ((*s == '/' || *s == ':' || *s == '\\') && s[1])
+ if ((*s == '/' || *s == '\\') && s[1])
rv = s + 1;
s++;
}
--
2.14.2
Re: [PATCH] cygcheck: Fix parsing of file names containing colons
On 10/25/2017 10:19 AM, Corinna Vinschen wrote:
On Oct 25 09:38, Ken Brown wrote:
On 10/25/2017 8:19 AM, Corinna Vinschen wrote:
On Oct 25 14:11, Corinna Vinschen wrote:
Hi Ken,
On Oct 25 07:23, Ken Brown wrote:
Up to now the function winsup/utils/dump_setup.cc:base skips past
colons when parsing file names. As a result, a line like
foo foo-1:2.3-4.tar.bz2 1
in /etc/setup/installed.db would cause 'cygcheck -cd foo' to report 4
as the installed version of foo insted of 1:2.3-4. This is not an
issue now, but it will become an issue when version numbers are
allowed to contain epochs.
---
winsup/utils/dump_setup.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/dump_setup.cc
index 320d69fab..3922b18f8 100644
--- a/winsup/utils/dump_setup.cc
+++ b/winsup/utils/dump_setup.cc
@@ -56,7 +56,7 @@ base (const char *s)
const char *rv = s;
while (*s)
{
- if ((*s == '/' || *s == ':' || *s == '\\') && s[1])
+ if ((*s == '/' || *s == '\\') && s[1])
I think this is a simplified way to test for the colon in paths like
C:/foo/bar. Nothing else makes sense in this context.
I'm not sure how much we care, but maybe we shoulkd fix the test to
ignore the colon only if it's the second character in the incoming
string?
Not "ignore", but "use as a delimiter" only as 2nd char in the input.
I'm not sure the distinction matters in this case, since the function is
just trying to get the base name. Anyway, how's the attached?
Fine, thanks.
But now that you mention it... why does parse_filename() call base() at
all? The filenames in installed.db are just basenames anyway. Does
that cover an older DB format we don't support anymore, perhaps?
It looks like parse_filename is more-or-less copied from the function
with the same name in the setup sources (in filemanip.cc). In that case
there might be a good reason to call base; I'll have to look more closely.
I just wonder now if we should simply remove base() and the call to it.
Either way, you're right, the colon check is just useless, so your first
patch was entirely sufficient.
What do you think? Stick to your patch or remove base()?
I vote for removing base. The attached patch does this.
Ken
From f679462937faf263de682c47f8d8e73b0c7e4136 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Tue, 24 Oct 2017 18:21:53 -0400
Subject: [PATCH] winsup/utils/dump_setup.cc: Remove the function 'base'
This was called only on filenames in /etc/setup/installed.db, which
are all basenames anyway. Moreover, base wasn't correctly handling
filenames containing colons.
---
winsup/utils/dump_setup.cc | 17 +
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/dump_setup.cc
index 320d69fab..4415954f9 100644
--- a/winsup/utils/dump_setup.cc
+++ b/winsup/utils/dump_setup.cc
@@ -48,21 +48,6 @@ find_tar_ext (const char *path)
return 0;
}
-static char *
-base (const char *s)
-{
- if (!s)
-return 0;
- const char *rv = s;
- while (*s)
-{
- if ((*s == '/' || *s == ':' || *s == '\\') && s[1])
- rv = s + 1;
- s++;
-}
- return (char *) rv;
-}
-
/* Parse a filename into package, version, and extension components. */
int
parse_filename (const char *in_fn, fileparse& f)
@@ -79,7 +64,7 @@ parse_filename (const char *in_fn, fileparse& f)
strcpy (f.tail, fn + n);
fn[n] = '\0';
f.pkg[0] = f.what[0] = '\0';
- p = base (fn);
+ p = fn;
for (ver = p; *ver; ver++)
if (*ver != '-')
continue;
--
2.14.2
Re: [PATCH] Add FAQ 4.46 How do I fix find_fast_cwd warnings?
On 11/12/2017 1:39 PM, Brian Inglis wrote: Having responded to some of these posts and being prompted by the suggestion in a reply to one by "Cyg simple", I attach an offering, in the off chance that anyone affected might actually check the FAQ or find it in a search. ;^> Even if they don't find it, we can refer them to the FAQ rather than re-writing variations on the same answer each time. But your patch should be to winsup/doc/faq-using.xml. Ken
Re: [PATCH] Add FAQ 4.46. How do I fix find_fast_cwd warnings?
On 11/12/2017 4:27 PM, Brian Inglis wrote: +Some ancient Cygwin releases asked users to report problems that were + difficult to diagnose to the mailing list with the message: + +find_fast_cwd: WARNING: Couldn't compute FAST_CWD pointer. Please report +this problem to the public mailing listcyg...@cygwin.com + +These problems were fixed long ago in updated Cygwin releases. The wording of the warning message was changed 3 years ago, in commit 0793492. I'm not sure that qualifies as ancient. I also don't think it's accurate to refer to the problem as "difficult to diagnose" or to say that the problems "were fixed long ago". The issue (Corinna will correct me if I'm wrong) is simply that new releases of Windows sometimes require changes in how Cygwin finds the fast_cwd pointer. So users of old versions of Cygwin on new versions of Windows might have problems, and this can certainly happen again in the future. But the FAQ doesn't need to go into that. Why not just say what the warning currently says (see path.cc:find_fast_cwd()): "This typically occurs if you're using an older Cygwin version on a newer Windows. Please update to the latest available Cygwin version from https://cygwin.com/. If the problem persists, please see https://cygwin.com/problems.html."; You can also add your sentence about contacting the vendor who provided the old Cygwin release. Ken
Re: [PATCH setup v4 6/6] Display area and location of official mirrors
On 12/10/2017 1:40 PM, Brian Inglis wrote: On 2017-12-10 10:49, Ken Brown wrote: Mirrors from mirrors.lst have area and location info, which we now display and add to the sort key. You didn't increase the list box width - are the hosts visible without scrolling? No, that was an oversight. I'll fix that locally and use it in future versions (or in the commit, if this version is accepted). Ken
Fix declaration of pthread_rwlock_* functions
The attached patch fixes the second problem reported in https://cygwin.com/ml/cygwin/2018-05/msg00316.html, though I'm not sure it's the right fix. Ken From 4940baac08cd9339d771d9db90a880c61610ae4c Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Wed, 30 May 2018 16:19:01 -0400 Subject: [PATCH] Declare the pthread_rwlock_* functions if __cplusplus >= 201402L Some of these functions are used in the C++ header. --- winsup/cygwin/include/pthread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/include/pthread.h b/winsup/cygwin/include/pthread.h index 3dfc2bc80..fed616532 100644 --- a/winsup/cygwin/include/pthread.h +++ b/winsup/cygwin/include/pthread.h @@ -187,7 +187,7 @@ int pthread_spin_unlock (pthread_spinlock_t *); #endif /* RW Locks */ -#if __XSI_VISIBLE >= 500 || __POSIX_VISIBLE >= 200112 +#if __XSI_VISIBLE >= 500 || __POSIX_VISIBLE >= 200112 || __cplusplus >= 201402L int pthread_rwlock_destroy (pthread_rwlock_t *rwlock); int pthread_rwlock_init (pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *attr); int pthread_rwlock_rdlock (pthread_rwlock_t *rwlock); -- 2.17.0
Re: Fix declaration of pthread_rwlock_* functions
On 6/1/2018 6:10 AM, Corinna Vinschen wrote: On May 30 16:28, Ken Brown wrote: The attached patch fixes the second problem reported in https://cygwin.com/ml/cygwin/2018-05/msg00316.html, though I'm not sure it's the right fix. Ken From 4940baac08cd9339d771d9db90a880c61610ae4c Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Wed, 30 May 2018 16:19:01 -0400 Subject: [PATCH] Declare the pthread_rwlock_* functions if __cplusplus >= 201402L Some of these functions are used in the C++ header. --- winsup/cygwin/include/pthread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/include/pthread.h b/winsup/cygwin/include/pthread.h index 3dfc2bc80..fed616532 100644 --- a/winsup/cygwin/include/pthread.h +++ b/winsup/cygwin/include/pthread.h @@ -187,7 +187,7 @@ int pthread_spin_unlock (pthread_spinlock_t *); #endif /* RW Locks */ -#if __XSI_VISIBLE >= 500 || __POSIX_VISIBLE >= 200112 +#if __XSI_VISIBLE >= 500 || __POSIX_VISIBLE >= 200112 || __cplusplus >= 201402L int pthread_rwlock_destroy (pthread_rwlock_t *rwlock); int pthread_rwlock_init (pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *attr); int pthread_rwlock_rdlock (pthread_rwlock_t *rwlock); -- 2.17.0 Pushed. Any text for winsup/cygwin/release/2.10.1, perhaps? Attached. Ken From 661edafedeb09d6ead1d3249763dc3fc6d3de40f Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Fri, 1 Jun 2018 08:31:43 -0400 Subject: [PATCH] Cygwin: Add pthread_rwlock_* fix to release notes --- winsup/cygwin/release/2.10.1 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/winsup/cygwin/release/2.10.1 b/winsup/cygwin/release/2.10.1 index c133124ae..42d9d1110 100644 --- a/winsup/cygwin/release/2.10.1 +++ b/winsup/cygwin/release/2.10.1 @@ -29,3 +29,6 @@ Bug Fixes - Fix a stack alignment problem which may lead to spurious crashes after fork. Addresses: https://cygwin.com/ml/cygwin-patches/2018-q2/msg00016.html + +- Fix a g++ compilation problem with -std=c++14 or -std=c++17. + Addresses: https://cygwin.com/ml/cygwin/2018-05/msg00316.html -- 2.17.0
[PATCH 0/5] Implement clearenv
This is a followup to https://cygwin.com/ml/cygwin/2018-05/msg00334.html. In this patch series I attempt to implement the glibc extension clearenv(). I also implement glibc's notion of environ==NULL being shorthand for an empty environment. Two questions: 1. I haven't yet absorbed what SIGFE means. I arbitrarily decorated clearenv with SIGFE rather than NOSIGFE in common.din, but I don't know if that's right. 2. I guarded the declaration of clearenv() with __GNU_VISIBLE, but again I'm not sure about this. On the one hand, clearenv() is a GNU extension, so __GNU_VISIBLE would seem to be the right guard. On the other hand, glibc declares clearenv() if _DEFAULT_SOURCE is defined, so maybe the guard should be relaxed if our goal is to emulate Linux. Yaakov? Ken Brown (5): Cygwin: Clarify some code in environ.cc Cygwin: Allow the environment pointer to be NULL Cygwin: Implement the GNU extension clearenv Cygwin: Remove workaround in environ.cc Cygwin: Document clearenv winsup/cygwin/common.din | 1 + winsup/cygwin/environ.cc | 45 ++ winsup/cygwin/include/cygwin/stdlib.h | 1 + winsup/cygwin/include/cygwin/version.h | 3 +- winsup/cygwin/release/2.10.1 | 1 + winsup/doc/posix.xml | 1 + 6 files changed, 44 insertions(+), 8 deletions(-) -- 2.17.0
[PATCH 5/5] Cygwin: Document clearenv
--- winsup/cygwin/include/cygwin/version.h | 3 ++- winsup/cygwin/release/2.10.1 | 1 + winsup/doc/posix.xml | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h index f08707eea..2991ab858 100644 --- a/winsup/cygwin/include/cygwin/version.h +++ b/winsup/cygwin/include/cygwin/version.h @@ -494,12 +494,13 @@ details. */ 323: scanf %l[ conversion. 324: Export sigtimedwait. 325: Export catclose, catgets, catopen. + 326: Export clearenv 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 325 +#define CYGWIN_VERSION_API_MINOR 326 /* There is also a compatibity version number associated with the shared memory regions. It is incremented when incompatible changes are made to the shared diff --git a/winsup/cygwin/release/2.10.1 b/winsup/cygwin/release/2.10.1 index 42d9d1110..ef7d08256 100644 --- a/winsup/cygwin/release/2.10.1 +++ b/winsup/cygwin/release/2.10.1 @@ -1,6 +1,7 @@ What's new: --- +- New API: clearenv. What changed: - diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml index 8b4bab1b0..0c95091ea 100644 --- a/winsup/doc/posix.xml +++ b/winsup/doc/posix.xml @@ -1284,6 +1284,7 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008). asprintf_r basename (see chapter "Implementation Notes") canonicalize_file_name +clearenv clog10 clog10f clog10l -- 2.17.0
[PATCH 3/5] Cygwin: Implement the GNU extension clearenv
---
winsup/cygwin/common.din | 1 +
winsup/cygwin/environ.cc | 20
winsup/cygwin/include/cygwin/stdlib.h | 1 +
3 files changed, 22 insertions(+)
diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 6e8bf9185..426cf172c 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -306,6 +306,7 @@ cimag NOSIGFE
cimagf NOSIGFE
cimagl NOSIGFE
cleanup_glue NOSIGFE
+clearenv SIGFE
clearerr SIGFE
clearerr_unlocked SIGFE
clock SIGFE
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 8e6bbe561..3676bd9ea 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -720,6 +720,26 @@ unsetenv (const char *name)
return -1;
}
+/* Clear the environment. */
+extern "C" int
+clearenv ()
+{
+ __try
+{
+ if (cur_environ () == lastenviron && lastenviron != NULL)
+ {
+ free (lastenviron);
+ lastenviron = NULL;
+ }
+ __cygwin_environ = NULL;
+ update_envptrs ();
+ return 0;
+}
+ __except (EFAULT) {}
+ __endtry
+ return -1;
+}
+
/* Minimal list of Windows vars which must be converted to uppercase.
Either for POSIX compatibility of for backward compatibility with
existing applications. */
diff --git a/winsup/cygwin/include/cygwin/stdlib.h
b/winsup/cygwin/include/cygwin/stdlib.h
index 845d2d81b..55d75e402 100644
--- a/winsup/cygwin/include/cygwin/stdlib.h
+++ b/winsup/cygwin/include/cygwin/stdlib.h
@@ -22,6 +22,7 @@ void setprogname (const char *);
#if __GNU_VISIBLE
char *canonicalize_file_name (const char *);
+int clearenv ();
#endif
#if __BSD_VISIBLE || __POSIX_VISIBLE >= 200112
int unsetenv (const char *);
--
2.17.0
[PATCH 4/5] Cygwin: Remove workaround in environ.cc
Commit ebd645e on 2001-10-03 made environ.cc:_addenv() add unneeded
space at the end of the environment block to "work around problems
with some buggy applications." This clutters the code and is
presumably no longer needed.
---
winsup/cygwin/environ.cc | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 3676bd9ea..7cdeded08 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -591,13 +591,11 @@ _addenv (const char *name, const char *value, int
overwrite)
{ /* Create new slot. */
int sz = envsize (cur_environ ());
- /* If sz == 0, we need two new slots, one for the terminating NULL.
-But we add two slots in all cases, as has been done since
-2001-10-03 (commit ebd645e) to "work around problems with
-some buggy applications." */
- int allocsz = (sz + 2) * sizeof (char *);
+ /* If sz == 0, we need two slots, one for the terminating NULL. */
+ int newsz = sz == 0 ? 2 : sz + 1;
+ int allocsz = newsz * sizeof (char *);
- offset = sz == 0 ? 0 : sz - 1;
+ offset = newsz - 2;
/* Allocate space for additional element. */
if (cur_environ () == lastenviron)
--
2.17.0
[PATCH 2/5] Cygwin: Allow the environment pointer to be NULL
Following glibc, interpret this as meaning the environment is empty.
---
winsup/cygwin/environ.cc | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index b452d21a5..8e6bbe561 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -483,6 +483,9 @@ my_findenv (const char *name, int *offset)
register char **p;
const char *c;
+ if (cur_environ () == NULL)
+return NULL;
+
c = name;
len = 0;
while (*c && *c != '=')
@@ -545,14 +548,18 @@ _getenv_r (struct _reent *, const char *name)
return findenv_func (name, &offset);
}
-/* Return size of environment block, including terminating NULL. */
+/* Return number of environment entries, including terminating NULL. */
static int __stdcall
envsize (const char * const *in_envp)
{
const char * const *envp;
+
+ if (in_envp == NULL)
+return 0;
+
for (envp = in_envp; *envp; envp++)
continue;
- return (1 + envp - in_envp) * sizeof (const char *);
+ return 1 + envp - in_envp;
}
/* Takes similar arguments to setenv except that overwrite is
@@ -584,23 +591,23 @@ _addenv (const char *name, const char *value, int
overwrite)
{ /* Create new slot. */
int sz = envsize (cur_environ ());
- /* Allocate space for two new slots even though only one is needed.
-According to the commit message for commit ebd645e
-(2001-10-03), this is done to "work around problems with some
-buggy applications." */
- int allocsz = sz + (2 * sizeof (char *));
+ /* If sz == 0, we need two new slots, one for the terminating NULL.
+But we add two slots in all cases, as has been done since
+2001-10-03 (commit ebd645e) to "work around problems with
+some buggy applications." */
+ int allocsz = (sz + 2) * sizeof (char *);
- offset = (sz - 1) / sizeof (char *);
+ offset = sz == 0 ? 0 : sz - 1;
/* Allocate space for additional element. */
if (cur_environ () == lastenviron)
lastenviron = __cygwin_environ = (char **) realloc (cur_environ (),
allocsz);
else if ((lastenviron = (char **) malloc (allocsz)) != NULL)
- __cygwin_environ = (char **) memcpy ((char **) lastenviron,
-__cygwin_environ, sz);
+ __cygwin_environ = (char **) memcpy (lastenviron, __cygwin_environ,
+sz * sizeof (char *));
- if (!__cygwin_environ)
+ if (!lastenviron)
{
#ifdef DEBUGGING
try_to_debug ();
--
2.17.0
[PATCH 1/5] Cygwin: Clarify some code in environ.cc
---
winsup/cygwin/environ.cc | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 43225341c..b452d21a5 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -545,6 +545,7 @@ _getenv_r (struct _reent *, const char *name)
return findenv_func (name, &offset);
}
+/* Return size of environment block, including terminating NULL. */
static int __stdcall
envsize (const char * const *in_envp)
{
@@ -582,11 +583,16 @@ _addenv (const char *name, const char *value, int
overwrite)
else
{ /* Create new slot. */
int sz = envsize (cur_environ ());
+
+ /* Allocate space for two new slots even though only one is needed.
+According to the commit message for commit ebd645e
+(2001-10-03), this is done to "work around problems with some
+buggy applications." */
int allocsz = sz + (2 * sizeof (char *));
offset = (sz - 1) / sizeof (char *);
- /* Allocate space for additional element plus terminating NULL. */
+ /* Allocate space for additional element. */
if (cur_environ () == lastenviron)
lastenviron = __cygwin_environ = (char **) realloc (cur_environ (),
allocsz);
--
2.17.0
Re: [PATCH 2/5] Cygwin: Allow the environment pointer to be NULL
On 6/4/2018 3:36 PM, Ken Brown wrote: Following glibc, interpret this as meaning the environment is empty. Sorry, please hold off on reviewing this patch. I just noticed that I missed at least one place (build_env()) where environ==NULL could cause a crash. I need to fix this and try to make sure there are no others. Ken
[PATCH v2 4/6] Cygwin: Remove workaround in environ.cc
Commit ebd645e on 2001-10-03 made environ.cc:_addenv() add unneeded
space at the end of the environment block to "work around problems
with some buggy applications." This clutters the code and is
presumably no longer needed.
---
winsup/cygwin/environ.cc | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index bbe539816..67ead1dde 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -592,13 +592,11 @@ _addenv (const char *name, const char *value, int
overwrite)
{ /* Create new slot. */
int sz = envsize (cur_environ ());
- /* If sz == 0, we need two new slots, one for the terminating NULL.
-But we add two slots in all cases, as has been done since
-2001-10-03 (commit ebd645e) to "work around problems with
-some buggy applications." */
- int allocsz = (sz + 2) * sizeof (char *);
+ /* If sz == 0, we need two new slots, one for the terminating NULL. */
+ int newsz = sz == 0 ? 2 : sz + 1;
+ int allocsz = newsz * sizeof (char *);
- offset = sz == 0 ? 0 : sz - 1;
+ offset = newsz - 2;
/* Allocate space for additional element. */
if (cur_environ () == lastenviron)
--
2.17.0
[PATCH v2 6/6] Bump Cygwin DLL version to 2.11.0
--- winsup/cygwin/include/cygwin/version.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h index 2991ab858..958acca3a 100644 --- a/winsup/cygwin/include/cygwin/version.h +++ b/winsup/cygwin/include/cygwin/version.h @@ -10,8 +10,8 @@ details. */ the Cygwin shared library". This version is used to track important changes to the DLL and is mainly informative in nature. */ -#define CYGWIN_VERSION_DLL_MAJOR 2010 -#define CYGWIN_VERSION_DLL_MINOR 1 +#define CYGWIN_VERSION_DLL_MAJOR 2011 +#define CYGWIN_VERSION_DLL_MINOR 0 /* Major numbers before CYGWIN_VERSION_DLL_EPOCH are incompatible. */ -- 2.17.0
[PATCH v2 2/6] Cygwin: Allow the environment pointer to be NULL
Following glibc, interpret this as meaning the environment is empty.
---
winsup/cygwin/environ.cc | 40 ++--
winsup/cygwin/pinfo.cc | 7 ---
2 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index b452d21a5..06e1ced01 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -30,6 +30,7 @@ details. */
#include "shared_info.h"
#include "ntdll.h"
+/* If this is not NULL, it points to memory allocated by us. */
static char **lastenviron;
/* Parse CYGWIN options */
@@ -483,6 +484,9 @@ my_findenv (const char *name, int *offset)
register char **p;
const char *c;
+ if (cur_environ () == NULL)
+return NULL;
+
c = name;
len = 0;
while (*c && *c != '=')
@@ -545,14 +549,18 @@ _getenv_r (struct _reent *, const char *name)
return findenv_func (name, &offset);
}
-/* Return size of environment block, including terminating NULL. */
+/* Return number of environment entries, including terminating NULL. */
static int __stdcall
envsize (const char * const *in_envp)
{
const char * const *envp;
+
+ if (in_envp == NULL)
+return 0;
+
for (envp = in_envp; *envp; envp++)
continue;
- return (1 + envp - in_envp) * sizeof (const char *);
+ return 1 + envp - in_envp;
}
/* Takes similar arguments to setenv except that overwrite is
@@ -584,23 +592,22 @@ _addenv (const char *name, const char *value, int
overwrite)
{ /* Create new slot. */
int sz = envsize (cur_environ ());
- /* Allocate space for two new slots even though only one is needed.
-According to the commit message for commit ebd645e
-(2001-10-03), this is done to "work around problems with some
-buggy applications." */
- int allocsz = sz + (2 * sizeof (char *));
+ /* If sz == 0, we need two new slots, one for the terminating NULL.
+But we add two slots in all cases, as has been done since
+2001-10-03 (commit ebd645e) to "work around problems with
+some buggy applications." */
+ int allocsz = (sz + 2) * sizeof (char *);
- offset = (sz - 1) / sizeof (char *);
+ offset = sz == 0 ? 0 : sz - 1;
/* Allocate space for additional element. */
if (cur_environ () == lastenviron)
- lastenviron = __cygwin_environ = (char **) realloc (cur_environ (),
+ lastenviron = __cygwin_environ = (char **) realloc (lastenviron,
allocsz);
- else if ((lastenviron = (char **) malloc (allocsz)) != NULL)
- __cygwin_environ = (char **) memcpy ((char **) lastenviron,
-__cygwin_environ, sz);
-
- if (!__cygwin_environ)
+ else if ((lastenviron = (char **) realloc (lastenviron, allocsz)) !=
NULL)
+ __cygwin_environ = (char **) memcpy (lastenviron, __cygwin_environ,
+sz * sizeof (char *));
+ if (!lastenviron)
{
#ifdef DEBUGGING
try_to_debug ();
@@ -1029,8 +1036,13 @@ build_env (const char * const *envp, PWCHAR &envblock,
int &envc,
char **dstp;
bool saw_spenv[SPENVS_SIZE] = {0};
+ static char *const empty_env[] = { NULL };
+
debug_printf ("envp %p", envp);
+ if (!envp)
+envp = empty_env;
+
/* How many elements? */
for (n = 0; envp[n]; n++)
continue;
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index e4eef8b3c..6b6986c9e 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -677,11 +677,12 @@ commune_process (void *arg)
sigproc_printf ("processing PICOM_ENVIRON");
unsigned n = 0;
char **env = cur_environ ();
- for (char **e = env; *e; e++)
- n += strlen (*e) + 1;
+ if (env)
+ for (char **e = env; *e; e++)
+ n += strlen (*e) + 1;
if (!WritePipeOverlapped (tothem, &n, sizeof n, &nr, 1000L))
sigproc_printf ("WritePipeOverlapped sizeof argv failed, %E");
- else
+ else if (env)
for (char **e = env; *e; e++)
if (!WritePipeOverlapped (tothem, *e, strlen (*e) + 1, &nr, 1000L))
{
--
2.17.0
[PATCH v2 3/6] Cygwin: Implement the GNU extension clearenv
---
winsup/cygwin/common.din | 1 +
winsup/cygwin/environ.cc | 20
winsup/cygwin/include/cygwin/stdlib.h | 3 +++
3 files changed, 24 insertions(+)
diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 6e8bf9185..426cf172c 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -306,6 +306,7 @@ cimag NOSIGFE
cimagf NOSIGFE
cimagl NOSIGFE
cleanup_glue NOSIGFE
+clearenv SIGFE
clearerr SIGFE
clearerr_unlocked SIGFE
clock SIGFE
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 06e1ced01..bbe539816 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -720,6 +720,26 @@ unsetenv (const char *name)
return -1;
}
+/* Clear the environment. */
+extern "C" int
+clearenv (void)
+{
+ __try
+{
+ if (cur_environ () == lastenviron)
+ {
+ free (lastenviron);
+ lastenviron = NULL;
+ }
+ __cygwin_environ = NULL;
+ update_envptrs ();
+ return 0;
+}
+ __except (EFAULT) {}
+ __endtry
+ return -1;
+}
+
/* Minimal list of Windows vars which must be converted to uppercase.
Either for POSIX compatibility of for backward compatibility with
existing applications. */
diff --git a/winsup/cygwin/include/cygwin/stdlib.h
b/winsup/cygwin/include/cygwin/stdlib.h
index 845d2d81b..e274b8b94 100644
--- a/winsup/cygwin/include/cygwin/stdlib.h
+++ b/winsup/cygwin/include/cygwin/stdlib.h
@@ -26,6 +26,9 @@ char *canonicalize_file_name (const char *);
#if __BSD_VISIBLE || __POSIX_VISIBLE >= 200112
int unsetenv (const char *);
#endif
+#if __MISC_VISIBLE
+int clearenv (void);
+#endif
#if __XSI_VISIBLE
char *ptsname (int);
int grantpt (int);
--
2.17.0
[PATCH v2 0/6] Implement clearenv
This is a followup to https://cygwin.com/ml/cygwin/2018-05/msg00334.html.
In this patch series I attempt to implement the glibc extension
clearenv(). I also implement glibc's notion of environ==NULL being
shorthand for an empty environment.
v2: In patch 2 I've tried harder to fix all the cases in which
environ==NULL could be a problem. I did this by grepping the sources
for 'cur_environ' and '__cygwin_environ', but it's still possible that
I missed something.
I've also incorporated the changes suggested by Corinna and Yaakov.
Ken Brown (6):
Cygwin: Clarify some code in environ.cc
Cygwin: Allow the environment pointer to be NULL
Cygwin: Implement the GNU extension clearenv
Cygwin: Remove workaround in environ.cc
Cygwin: Document clearenv and bump API minor
Bump Cygwin DLL version to 2.11.0
winsup/cygwin/common.din | 1 +
winsup/cygwin/environ.cc | 56 +++-
winsup/cygwin/include/cygwin/stdlib.h| 3 ++
winsup/cygwin/include/cygwin/version.h | 7 +--
winsup/cygwin/pinfo.cc | 7 +--
winsup/cygwin/release/{2.10.1 => 2.11.0} | 1 +
winsup/doc/new-features.xml | 20 +
winsup/doc/posix.xml | 1 +
8 files changed, 80 insertions(+), 16 deletions(-)
rename winsup/cygwin/release/{2.10.1 => 2.11.0} (97%)
--
2.17.0
[PATCH v2 1/6] Cygwin: Clarify some code in environ.cc
---
winsup/cygwin/environ.cc | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 43225341c..b452d21a5 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -545,6 +545,7 @@ _getenv_r (struct _reent *, const char *name)
return findenv_func (name, &offset);
}
+/* Return size of environment block, including terminating NULL. */
static int __stdcall
envsize (const char * const *in_envp)
{
@@ -582,11 +583,16 @@ _addenv (const char *name, const char *value, int
overwrite)
else
{ /* Create new slot. */
int sz = envsize (cur_environ ());
+
+ /* Allocate space for two new slots even though only one is needed.
+According to the commit message for commit ebd645e
+(2001-10-03), this is done to "work around problems with some
+buggy applications." */
int allocsz = sz + (2 * sizeof (char *));
offset = (sz - 1) / sizeof (char *);
- /* Allocate space for additional element plus terminating NULL. */
+ /* Allocate space for additional element. */
if (cur_environ () == lastenviron)
lastenviron = __cygwin_environ = (char **) realloc (cur_environ (),
allocsz);
--
2.17.0
[PATCH v2 5/6] Cygwin: Document clearenv and bump API minor
Also add earlier "What changed" items to new-features.xml.
---
winsup/cygwin/include/cygwin/version.h | 3 ++-
winsup/cygwin/release/{2.10.1 => 2.11.0} | 1 +
winsup/doc/new-features.xml | 20
winsup/doc/posix.xml | 1 +
4 files changed, 24 insertions(+), 1 deletion(-)
rename winsup/cygwin/release/{2.10.1 => 2.11.0} (97%)
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index f08707eea..2991ab858 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -494,12 +494,13 @@ details. */
323: scanf %l[ conversion.
324: Export sigtimedwait.
325: Export catclose, catgets, catopen.
+ 326: Export clearenv
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 325
+#define CYGWIN_VERSION_API_MINOR 326
/* There is also a compatibity version number associated with the shared memory
regions. It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/release/2.10.1 b/winsup/cygwin/release/2.11.0
similarity index 97%
rename from winsup/cygwin/release/2.10.1
rename to winsup/cygwin/release/2.11.0
index 42d9d1110..ef7d08256 100644
--- a/winsup/cygwin/release/2.10.1
+++ b/winsup/cygwin/release/2.11.0
@@ -1,6 +1,7 @@
What's new:
---
+- New API: clearenv.
What changed:
-
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 59af6efc0..5029b7c08 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -4,6 +4,26 @@
What's new and what changed in Cygwin
+What's new and what changed in 2.11
+
+
+
+
+SO_RCVTIMEO and SO_SNDTIMEO socket options are now honored.
+
+
+
+/proc/cpuinfo now reports L3 cache size on Intel CPUs.
+
+
+
+New API: clearenv.
+
+
+
+
+
+
What's new and what changed in 2.10
diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml
index 8b4bab1b0..0c95091ea 100644
--- a/winsup/doc/posix.xml
+++ b/winsup/doc/posix.xml
@@ -1284,6 +1284,7 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008).
asprintf_r
basename (see chapter "Implementation Notes")
canonicalize_file_name
+clearenv
clog10
clog10f
clog10l
--
2.17.0
Re: getfacl output
[Redirecting to cygwin-patches.]
On 7/23/2018 11:06 AM, Corinna Vinschen wrote:
On Jul 23 10:43, Ken Brown wrote:
This is obviously very minor, but I bumped into it because of a failing
emacs test.
Cygwin's getfacl prints only one colon after "mask" and "other", but Linux's
prints two. I'm sure this was done for a reason, but I'm wondering if it
would be better to follow Linux.
The original version was designed after Solaris documentation,
but the layout is supposed to look like Linux for a while, so
ther missing colon is a bug.
I'll be glad to submit a patch.
Glad to review it :)
Attached.
Ken
From de841bf40ecdbf93e87d0900d4dd567af32b072f Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Mon, 23 Jul 2018 10:10:03 -0400
Subject: [PATCH] getfacl and setfacl: Align with Linux
Make getfacl print two colons instead of one after "other" and "mask".
Change the help text for setfacl to indicate that there can be either
one colon or two.
---
winsup/utils/getfacl.c | 12 ++--
winsup/utils/setfacl.c | 6 +++---
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/winsup/utils/getfacl.c b/winsup/utils/getfacl.c
index 57c58fb6a..363226d6e 100644
--- a/winsup/utils/getfacl.c
+++ b/winsup/utils/getfacl.c
@@ -98,14 +98,14 @@ usage (FILE * stream)
" user:name or uid:perm\n"
" group::perm\n"
" group:name or gid:perm\n"
- " mask:perm\n"
- " other:perm\n"
+ " mask::perm\n"
+ " other::perm\n"
" default:user::perm\n"
" default:user:name or uid:perm\n"
" default:group::perm\n"
" default:group:name or gid:perm\n"
- " default:mask:perm\n"
- " default:other:perm\n"
+ " default:mask::perm\n"
+ " default:other::perm\n"
"\n");
}
}
@@ -265,10 +265,10 @@ main (int argc, char **argv)
n += printf ("group:%s:", groupname (acls[i].a_id));
break;
case CLASS_OBJ:
- printf ("mask:");
+ printf ("mask::");
break;
case OTHER_OBJ:
- printf ("other:");
+ printf ("other::");
break;
}
n += printf ("%s", permstr (acls[i].a_perm));
diff --git a/winsup/utils/setfacl.c b/winsup/utils/setfacl.c
index 373265bf0..2577ab776 100644
--- a/winsup/utils/setfacl.c
+++ b/winsup/utils/setfacl.c
@@ -119,7 +119,7 @@ getaclentry (action_t action, char *c, aclent_t *ace)
}
else if (!(ace->a_type & (USER_OBJ | GROUP_OBJ)))
{
- /* Mask and other entries may contain an extra colon. */
+ /* Mask and other entries may contain one or two colons. */
if (*c == ':')
++c;
}
@@ -558,8 +558,8 @@ usage (FILE *stream)
"u[ser]:uid:perm\n"
"g[roup]::perm\n"
"g[roup]:gid:perm\n"
-"m[ask]:perm\n"
-"o[ther]:perm\n"
+"m[ask]:[:]perm\n"
+"o[ther]:[:]perm\n"
"\n"
" Default entries are like the above with the additional default
identifier.\n"
" For example: \n"
--
2.17.0
Re: getfacl output
On 7/23/2018 11:37 AM, Corinna Vinschen wrote: On Jul 23 11:15, Ken Brown wrote: [Redirecting to cygwin-patches.] On 7/23/2018 11:06 AM, Corinna Vinschen wrote: On Jul 23 10:43, Ken Brown wrote: This is obviously very minor, but I bumped into it because of a failing emacs test. Cygwin's getfacl prints only one colon after "mask" and "other", but Linux's prints two. I'm sure this was done for a reason, but I'm wondering if it would be better to follow Linux. The original version was designed after Solaris documentation, but the layout is supposed to look like Linux for a while, so ther missing colon is a bug. I'll be glad to submit a patch. Glad to review it :) Attached. Pushed. I just wonder if we shouldn't simplify getfacl to use acl_to_text instead. Yes, that makes sense. I'll take a look. Ken
Re: getfacl output
On 7/23/2018 12:06 PM, Ken Brown wrote:
On 7/23/2018 11:37 AM, Corinna Vinschen wrote:
On Jul 23 11:15, Ken Brown wrote:
[Redirecting to cygwin-patches.]
On 7/23/2018 11:06 AM, Corinna Vinschen wrote:
On Jul 23 10:43, Ken Brown wrote:
This is obviously very minor, but I bumped into it because of a
failing
emacs test.
Cygwin's getfacl prints only one colon after "mask" and "other",
but Linux's
prints two. I'm sure this was done for a reason, but I'm wondering
if it
would be better to follow Linux.
The original version was designed after Solaris documentation,
but the layout is supposed to look like Linux for a while, so
ther missing colon is a bug.
I'll be glad to submit a patch.
Glad to review it :)
Attached.
Pushed. I just wonder if we shouldn't simplify getfacl to use
acl_to_text instead.
Yes, that makes sense. I'll take a look.
Patch attached.
I thought it might be possible to simplify setfacl in a similar way, but
I didn't see a way to do it.
Ken
From 89cc58e5c4787e4a01b6938256cc2fa0101d0313 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Mon, 23 Jul 2018 17:46:41 -0400
Subject: [PATCH] getfacl: Simplify by using acl_to_any_text
---
winsup/utils/getfacl.c | 153 +++--
1 file changed, 41 insertions(+), 112 deletions(-)
diff --git a/winsup/utils/getfacl.c b/winsup/utils/getfacl.c
index 363226d6e..08dfc0bf3 100644
--- a/winsup/utils/getfacl.c
+++ b/winsup/utils/getfacl.c
@@ -14,6 +14,8 @@ details. */
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -21,18 +23,6 @@ details. */
static char *prog_name;
-char *
-permstr (mode_t perm)
-{
- static char pbuf[4];
-
- pbuf[0] = (perm & S_IROTH) ? 'r' : '-';
- pbuf[1] = (perm & S_IWOTH) ? 'w' : '-';
- pbuf[2] = (perm & S_IXOTH) ? 'x' : '-';
- pbuf[3] = '\0';
- return pbuf;
-}
-
const char *
username (uid_t uid)
{
@@ -150,9 +140,9 @@ main (int argc, char **argv)
int eopt = 0;
int dopt = 0;
int nopt = 0;
+ int options = 0;
int istty = isatty (fileno (stdout));
struct stat st;
- aclent_t acls[MAX_ACL_ENTRIES];
prog_name = program_invocation_short_name;
@@ -192,19 +182,26 @@ main (int argc, char **argv)
usage (stderr);
return 1;
}
+ if (nopt)
+options |= TEXT_NUMERIC_IDS;
+ if (eopt > 0)
+options |= TEXT_ALL_EFFECTIVE;
+ else if (!eopt)
+options |= TEXT_SOME_EFFECTIVE;
+ if (istty)
+options |= TEXT_SMART_INDENT;
for (; optind < argc; ++optind)
{
- int i, num_acls;
- mode_t mask = S_IRWXO, def_mask = S_IRWXO;
+ acl_t access_acl = NULL, default_acl = NULL;
+ char *access_txt, *default_txt;
if (stat (argv[optind], &st)
- || (num_acls = acl (argv[optind], GETACL, MAX_ACL_ENTRIES, acls)) < 0)
- {
- fprintf (stderr, "%s: %s: %s\n",
- prog_name, argv[optind], strerror (errno));
- ret = 2;
- continue;
- }
+ || (!dopt
+ && !(access_acl = acl_get_file (argv[optind], ACL_TYPE_ACCESS)))
+ || (!aopt && S_ISDIR (st.st_mode)
+ && !(default_acl = acl_get_file (argv[optind],
+ ACL_TYPE_DEFAULT
+ goto err;
if (!copt)
{
printf ("# file: %s\n", argv[optind]);
@@ -223,103 +220,35 @@ main (int argc, char **argv)
(st.st_mode & S_ISGID) ? 's' : '-',
(st.st_mode & S_ISVTX) ? 't' : '-');
}
- for (i = 0; i < num_acls; ++i)
- {
- if (acls[i].a_type == CLASS_OBJ)
- mask = acls[i].a_perm;
- else if (acls[i].a_type == DEF_CLASS_OBJ)
- def_mask = acls[i].a_perm;
- }
- for (i = 0; i < num_acls; ++i)
+ if (access_acl)
{
- int n = 0;
- int print_effective = 0;
- mode_t effective = acls[i].a_perm;
-
- if (acls[i].a_type & ACL_DEFAULT)
- {
- if (aopt)
- continue;
- n += printf ("default:");
- }
- else if (dopt)
- continue;
- switch (acls[i].a_type & ~ACL_DEFAULT)
+ if (!(access_txt = acl_to_any_text (access_acl, NULL, '\n', options)))
{
- case USER_OBJ:
- printf ("user::");
- break;
- case USER:
- if (nopt)
- n += printf ("user:%lu:", (unsigned long)acls[i].a_id);
- else
- n += printf ("user:%s:", username (acls[i].a_id));
- break;
- case GROUP_OBJ:
- n += printf ("g
[PATCH] setfacl: Rename the option --file to --set-file, as on Linux
Retain --file as an undocumented option for backwards compatibility. --- winsup/cygwin/release/2.11.0 | 2 ++ winsup/doc/new-features.xml | 4 winsup/doc/utils.xml | 4 ++-- winsup/utils/setfacl.c | 5 +++-- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/winsup/cygwin/release/2.11.0 b/winsup/cygwin/release/2.11.0 index 2d86dea3f..171b5d4d0 100644 --- a/winsup/cygwin/release/2.11.0 +++ b/winsup/cygwin/release/2.11.0 @@ -18,6 +18,8 @@ What changed: - Drop denormal-operand exception from FE_ALL_EXCEPT, as on Linux. +- Rename the --file option of setfacl(1) to --set-file, as on Linux. + Bug Fixes - diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml index ad45a56e0..0107f75e2 100644 --- a/winsup/doc/new-features.xml +++ b/winsup/doc/new-features.xml @@ -30,6 +30,10 @@ New Header:. Drop denormal-operand exception from FE_ALL_EXCEPT, as on Linux. + +Rename the --file option of setfacl(1) to --set-file, as on Linux. + + diff --git a/winsup/doc/utils.xml b/winsup/doc/utils.xml index 948db5832..182134379 100644 --- a/winsup/doc/utils.xml +++ b/winsup/doc/utils.xml @@ -1968,7 +1968,7 @@ setfacl [-n] {[-bk]|[-x acl_entries] [-m acl_entries]} FILE... -b, --remove-all remove all extended ACL entries\n" -x, --delete delete one or more specified ACL entries\n" - -f, --file set ACL entries for FILE to ACL entries read\n" + -f, --set-file set ACL entries for FILE to ACL entries read\n" from ACL_FILE\n" -k, --remove-default remove all default ACL entries\n" -m, --modify modify one or more specified ACL entries\n" @@ -2035,7 +2035,7 @@ At least one of (-b, -x, -f, -k, -m, -s) must be specified\n" d[efault]:o[ther][:] - -f,--file Take the Acl_entries + -f,--set-file Take the Acl_entries from ACL_FILE one per line. Whitespace characters are ignored, and the character "#" may be used to start a comment. The special filename "-" indicates reading from stdin. Note that you can use this with diff --git a/winsup/utils/setfacl.c b/winsup/utils/setfacl.c index 2577ab776..926581a39 100644 --- a/winsup/utils/setfacl.c +++ b/winsup/utils/setfacl.c @@ -536,7 +536,7 @@ usage (FILE *stream) "\n" " -b, --remove-all remove all extended ACL entries\n" " -x, --delete delete one or more specified ACL entries\n" -" -f, --file set ACL entries for FILE to ACL entries read\n" +" -f, --set-file set ACL entries for FILE to ACL entries read\n" " from ACL_FILE\n" " -k, --remove-default remove all default ACL entries\n" " -m, --modify modify one or more specified ACL entries\n" @@ -595,7 +595,7 @@ usage (FILE *stream) "d[efault]:m[ask][:]\n" "d[efault]:o[ther][:]\n" "\n" -"-f, --file\n" +"-f, --set-file\n" " Take the Acl_entries from ACL_FILE one per line. Whitespace characters are\n" " ignored, and the character \"#\" may be used to start a comment. The special\n" " filename \"-\" indicates reading from stdin.\n" @@ -652,6 +652,7 @@ usage (FILE *stream) struct option longopts[] = { {"remove-all", no_argument, NULL, 'b'}, {"delete", required_argument, NULL, 'x'}, + {"set-file", required_argument, NULL, 'f'}, {"file", required_argument, NULL, 'f'}, {"remove-default", no_argument, NULL, 'k'}, {"modify", required_argument, NULL, 'm'}, -- 2.17.0
[PATCH] Cygwin: af_unix_spinlock_t: add initializer
Also fix a typo.
---
winsup/cygwin/fhandler.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index d02b9a913..7e460701c 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -832,9 +832,10 @@ class fhandler_socket_local: public fhandler_socket_wsock
/* Sharable spinlock with low CPU profile. These locks are NOT recursive! */
class af_unix_spinlock_t
{
- LONG locked; /* 0 oder 1 */
+ LONG locked; /* 0 or 1 */
public:
+ af_unix_spinlock_t () : locked (0) {}
void lock ()
{
LONG ret = InterlockedExchange (&locked, 1);
--
2.17.0
Re: [PATCH] Cygwin: af_unix_spinlock_t: add initializer
On 1/10/2019 1:02 PM, Corinna Vinschen wrote:
> On Jan 10 17:56, Ken Brown wrote:
>> Also fix a typo.
>> ---
>> winsup/cygwin/fhandler.h | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
>> index d02b9a913..7e460701c 100644
>> --- a/winsup/cygwin/fhandler.h
>> +++ b/winsup/cygwin/fhandler.h
>> @@ -832,9 +832,10 @@ class fhandler_socket_local: public
>> fhandler_socket_wsock
>> /* Sharable spinlock with low CPU profile. These locks are NOT recursive!
>> */
>> class af_unix_spinlock_t
>> {
>> - LONG locked; /* 0 oder 1 */
>> + LONG locked; /* 0 or 1 */
>
> Huh.
>
>> public:
>> + af_unix_spinlock_t () : locked (0) {}
>
> Why do we need that? The spinlock is created as part of a shared mem
> region which gets initialized to all zero, no? Or do you plan to use it
> outside of this scenario?
At the moment I'm using it in the new FIFO code, and I'm not sure yet whether
it
will eventually be in shared memory. (Until I get things working, I'm
postponing thinking about whether I need shared memory.)
Would it be better to use some other kind of spinlock until I know for sure
that
I need shared memory? My only reason for choosing af_unix_spinlock_t is that I
was copying code from fhandler_socket_unix, and this saved me the trouble of
learning about other kinds of spinlocks.
Ken
Re: [PATCH] Cygwin: af_unix_spinlock_t: add initializer
On 1/10/2019 3:16 PM, Corinna Vinschen wrote:
> On Jan 10 18:36, Ken Brown wrote:
>> On 1/10/2019 1:02 PM, Corinna Vinschen wrote:
>>> On Jan 10 17:56, Ken Brown wrote:
>>>> Also fix a typo.
>>>> ---
>>>>winsup/cygwin/fhandler.h | 3 ++-
>>>>1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
>>>> index d02b9a913..7e460701c 100644
>>>> --- a/winsup/cygwin/fhandler.h
>>>> +++ b/winsup/cygwin/fhandler.h
>>>> @@ -832,9 +832,10 @@ class fhandler_socket_local: public
>>>> fhandler_socket_wsock
>>>>/* Sharable spinlock with low CPU profile. These locks are NOT
>>>> recursive! */
>>>>class af_unix_spinlock_t
>>>>{
>>>> - LONG locked; /* 0 oder 1 */
>>>> + LONG locked; /* 0 or 1 */
>>>
>>> Huh.
>>>
>>>>public:
>>>> + af_unix_spinlock_t () : locked (0) {}
>>>
>>> Why do we need that? The spinlock is created as part of a shared mem
>>> region which gets initialized to all zero, no? Or do you plan to use it
>>> outside of this scenario?
>>
>> At the moment I'm using it in the new FIFO code, and I'm not sure yet
>> whether it
>> will eventually be in shared memory. (Until I get things working, I'm
>> postponing thinking about whether I need shared memory.)
>>
>> Would it be better to use some other kind of spinlock until I know for sure
>> that
>> I need shared memory? My only reason for choosing af_unix_spinlock_t is
>> that I
>> was copying code from fhandler_socket_unix, and this saved me the trouble of
>> learning about other kinds of spinlocks.
>
> The above patch shouldn't hurt in the least since it's not used anyway
> when allocating the shared mem region used by the AF_UNIX socket code.
> If it helps you, I can push it, no problem.
>
> Just make sure this spinlock is the right thing for you. The idea here
> was to have a fast, sharable(*) lock without context switching, only
> guarding small code blocks which don't hang due to resource starving.
>
> If you have to guard more complex code chunks, it might be better to
> use a kernel locking object like mutex or semaphore.
>
>
> So, push or no push?
Please push. Thanks.
Ken
[PATCH fifo 0/8] Allow a FIFO to have multiple writers
Currently a FIFO can have only one writer. A second attempt to open the FIFO for writing blocks while fhandler_fifo::open waits for the read_ready event to be signalled. This patch series tries to fix the problem by having the reader open multiple instances of the Windows named pipe underlying the FIFO. When the FIFO is opened for reading, a 'listen_client' thread is created that listens for clients (writers) to connect to the pipe. It creates new pipe instances as needed. fhandler_fifo::raw_read loops through the connected writers, checking for input. I've tested it by running the fifo client and server programs from Chapter 44 of the book "The Linux Programming Interface: Linux and UNIX System Programming Handbook" by Michael Kerrisk. (See https://cygwin.com/ml/cygwin/2015-03/msg00047.html for simplified versions of these programs.) These work as on Linux. I've also tried the test given in http://www.cygwin.org/ml/cygwin/2015-12/msg00311.html. It works as on Linux also. TODO: - Try to get the code to work for duplexers (FIFOs opened for reading and writing). I haven't thought about this at all yet. - Think about what it would take to allow multiple readers. I'm not very optimistic about this, but my impression is that the multiple-writer case is more important in practice. Ken Ken Brown (8): Cygwin: FIFO: stop using overlapped I/O Cygwin: FIFO: allow multiple writers Cygwin: FIFO: add a spinlock Cygwin: FIFO: improve EOF detection Cygwin: FIFO: update clone and dup Cygwin: FIFO: update fixup_after_fork Cygwin: FIFO: update set_close_on_exec Cygwin: FIFO: update select winsup/cygwin/fhandler.h | 58 ++- winsup/cygwin/fhandler_fifo.cc | 732 +++-- winsup/cygwin/select.cc| 161 +++- winsup/cygwin/select.h | 7 + 4 files changed, 819 insertions(+), 139 deletions(-) -- 2.17.0
[PATCH fifo 1/8] Cygwin: FIFO: stop using overlapped I/O
Make fhandler_fifo a derived class of fhandler_base instead of
fhandler_base_overlapped.
Replace the create_pipe macro, which is based on
fhandler_pipe::create, by new create_pipe and open_pipe methods.
These use NT functions instead of Win32 functions. Replace fifo_name
by get_pipe_name, which returns a pointer to a UNICODE_STRING.
Remove the fnevent macro, which would now be needed only once.
Add a raw_write method, adapted from fhandler_base::raw_write.
Adapt all functions to the changes above.
---
winsup/cygwin/fhandler.h | 13 +-
winsup/cygwin/fhandler_fifo.cc | 337 ++---
2 files changed, 236 insertions(+), 114 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 0da87e985..57e97c277 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1234,14 +1234,21 @@ public:
}
};
-class fhandler_fifo: public fhandler_base_overlapped
+#define CYGWIN_FIFO_PIPE_NAME_LEN 47
+
+class fhandler_fifo: public fhandler_base
{
HANDLE read_ready;
HANDLE write_ready;
+ UNICODE_STRING pipe_name;
+ WCHAR pipe_name_buf[CYGWIN_FIFO_PIPE_NAME_LEN + 1];
bool __reg2 wait (HANDLE);
- char __reg2 *fifo_name (char *, const char *);
+ NTSTATUS npfs_handle (HANDLE &);
+ HANDLE create_pipe ();
+ NTSTATUS open_pipe ();
public:
fhandler_fifo ();
+ PUNICODE_STRING get_pipe_name ();
int open (int, mode_t);
off_t lseek (off_t offset, int whence);
int close ();
@@ -1249,6 +1256,7 @@ public:
bool isfifo () const { return true; }
void set_close_on_exec (bool val);
void __reg3 raw_read (void *ptr, size_t& ulen);
+ ssize_t __reg3 raw_write (const void *ptr, size_t ulen);
bool arm (HANDLE h);
void fixup_after_fork (HANDLE);
int __reg2 fstatvfs (struct statvfs *buf);
@@ -1262,7 +1270,6 @@ public:
{
x->pc.free_strings ();
*reinterpret_cast (x) = *this;
-reinterpret_cast (x)->atomic_write_buf = NULL;
x->reset (this);
}
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 5733ec778..cb269e344 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -7,6 +7,7 @@
details. */
#include "winsup.h"
+#include
#include "miscfuncs.h"
#include "cygerrno.h"
@@ -21,26 +22,32 @@
#include "ntdll.h"
#include "cygwait.h"
+/* This is only to be used for writers. When reading,
+STATUS_PIPE_EMPTY simply means there's no data to be read. */
+#define STATUS_PIPE_IS_CLOSED(status) \
+ ({ NTSTATUS _s = (status); \
+ _s == STATUS_PIPE_CLOSING \
+ || _s == STATUS_PIPE_BROKEN \
+ || _s == STATUS_PIPE_EMPTY; })
+
fhandler_fifo::fhandler_fifo ():
- fhandler_base_overlapped (),
+ fhandler_base (),
read_ready (NULL), write_ready (NULL)
{
- max_atomic_write = DEFAULT_PIPEBUFSIZE;
+ pipe_name_buf[0] = L'\0';
need_fork_fixup (true);
}
-#define fnevent(w) fifo_name (npbuf, w "-event")
-#define fnpipe() fifo_name (npbuf, "fifo")
-#define create_pipe(r, w) \
- fhandler_pipe::create (sa_buf, (r), (w), 0, fnpipe (), open_mode)
-
-char *
-fhandler_fifo::fifo_name (char *buf, const char *what)
+PUNICODE_STRING
+fhandler_fifo::get_pipe_name ()
{
- /* Generate a semi-unique name to associate with this fifo. */
- __small_sprintf (buf, "%s.%08x.%016X", what, get_dev (),
- get_ino ());
- return buf;
+ if (!pipe_name_buf[0])
+{
+ __small_swprintf (pipe_name_buf, L"%S-fifo.%08x.%016X",
+ &cygheap->installation_key, get_dev (), get_ino ());
+ RtlInitUnicodeString (&pipe_name, pipe_name_buf);
+}
+ return &pipe_name;
}
inline PSECURITY_ATTRIBUTES
@@ -56,10 +63,8 @@ fhandler_fifo::arm (HANDLE h)
const char *what;
if (h == read_ready)
what = "reader";
- else if (h == write_ready)
-what = "writer";
else
-what = "overlapped event";
+what = "writer";
debug_only_printf ("arming %s", what);
#endif
@@ -73,17 +78,113 @@ fhandler_fifo::arm (HANDLE h)
return res;
}
+NTSTATUS
+fhandler_fifo::npfs_handle (HANDLE &nph)
+{
+ static NO_COPY SRWLOCK npfs_lock;
+ static NO_COPY HANDLE npfs_dirh;
+
+ NTSTATUS status = STATUS_SUCCESS;
+ OBJECT_ATTRIBUTES attr;
+ IO_STATUS_BLOCK io;
+
+ /* Lockless after first call. */
+ if (npfs_dirh)
+{
+ nph = npfs_dirh;
+ return STATUS_SUCCESS;
+}
+ AcquireSRWLockExclusive (&npfs_lock);
+ if (!npfs_dirh)
+{
+ InitializeObjectAttributes (&attr, &ro_u_npfs, 0, NULL, NULL);
+ status = NtOpenFile (&npfs_dirh, FILE_READ_ATTRIBUTES | SYNCHRONIZE,
+ &attr, &io, FILE_SHARE_READ | FILE_SHARE_WRITE,
+ 0);
+}
+ ReleaseSRWLockExclusive (&npfs_lock);
+ if (NT_SUCCESS (status))
+nph = npfs_dirh;
+ return status;
+}
+
+/* Called when pipe is opened for reading. */
+HANDLE
+fhandler_fifo::create_pipe ()
+{
+ NTSTATUS status;
+ HANDLE npfsh;
+ HANDLE ph = NULL;
+ A
[PATCH fifo 3/8] Cygwin: FIFO: add a spinlock
Don't let listen_client_thread and raw_read access the client list
simultaneously.
---
winsup/cygwin/fhandler.h | 3 +++
winsup/cygwin/fhandler_fifo.cc | 34 +-
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index e7c4af6a1..997dc0b6d 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1267,6 +1267,7 @@ class fhandler_fifo: public fhandler_base
WCHAR pipe_name_buf[CYGWIN_FIFO_PIPE_NAME_LEN + 1];
fifo_client_handler client[MAX_CLIENTS];
int nclients, nconnected;
+ af_unix_spinlock_t _fifo_client_lock;
bool __reg2 wait (HANDLE);
NTSTATUS npfs_handle (HANDLE &);
HANDLE create_pipe_instance (bool);
@@ -1278,6 +1279,8 @@ public:
fhandler_fifo ();
PUNICODE_STRING get_pipe_name ();
DWORD listen_client_thread ();
+ void fifo_client_lock () { _fifo_client_lock.lock (); }
+ void fifo_client_unlock () { _fifo_client_lock.unlock (); }
int open (int, mode_t);
off_t lseek (off_t offset, int whence);
int close ();
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index e91e88050..b0016ee90 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -349,13 +349,17 @@ fhandler_fifo::listen_client_thread ()
int i;
DWORD wait_ret;
+ fifo_client_lock ();
found = false;
for (i = 0; i < nclients; i++)
switch (client[i].state)
{
case fc_invalid:
if (disconnect_and_reconnect (i) < 0)
- goto errout;
+ {
+ fifo_client_unlock ();
+ goto errout;
+ }
/* Fall through. */
case fc_connected:
w[i] = client[i].dummy_evt;
@@ -369,13 +373,15 @@ fhandler_fifo::listen_client_thread ()
break;
}
w[nclients] = lct_termination_evt;
+ int res = 0;
if (!found)
- {
- if (add_client () < 0)
- goto errout;
- else
- continue;
- }
+ res = add_client ();
+ fifo_client_unlock ();
+ if (res < 0)
+ goto errout;
+ else if (!found)
+ continue;
+
if (!arm (read_ready))
{
__seterrno ();
@@ -391,9 +397,11 @@ fhandler_fifo::listen_client_thread ()
return 0;
else
{
+ fifo_client_lock ();
client[i].state = fc_connected;
nconnected++;
set_pipe_non_blocking (client[i].fh->get_handle (), true);
+ fifo_client_unlock ();
yield ();
}
}
@@ -664,6 +672,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
}
/* Poll the connected clients for input. */
+ fifo_client_lock ();
for (int i = 0; i < nclients; i++)
if (client[i].state == fc_connected)
{
@@ -671,15 +680,22 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
client[i].fh->fhandler_base::raw_read (in_ptr, len);
ssize_t nread = (ssize_t) len;
if (nread > 0)
- return;
+ {
+ fifo_client_unlock ();
+ return;
+ }
else if (nread < 0 && GetLastError () != ERROR_NO_DATA)
- goto errout;
+ {
+ fifo_client_unlock ();
+ goto errout;
+ }
else if (nread == 0) /* Client has disconnected. */
{
client[i].state = fc_invalid;
nconnected--;
}
}
+ fifo_client_unlock ();
if (is_nonblocking ())
{
set_errno (EAGAIN);
--
2.17.0
[PATCH fifo 8/8] Cygwin: FIFO: update select
Add static functions peek_fifo, thread_fifo, start_thread_fifo, and
fifo_cleanup to select.cc. These are based on the corresponding pipe
functions, the main difference being that peek_fifo loops through the
connected clients to see if any of them have data available for
reading.
Add the fhandler_fifo methods select_read, select_write, and
select_except.
Add accessor methods get_nclients, get_handle, and is_connected that
are needed by peek_fifo.
---
winsup/cygwin/fhandler.h | 4 +
winsup/cygwin/select.cc | 161 +++
winsup/cygwin/select.h | 7 ++
3 files changed, 157 insertions(+), 15 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 0ebc44e0d..f6982f0ba 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1278,6 +1278,10 @@ class fhandler_fifo: public fhandler_base
public:
fhandler_fifo ();
bool hit_eof ();
+ int get_nclients () const { return nclients; }
+ HANDLE& get_handle () { return fhandler_base::get_handle (); }
+ HANDLE get_handle (int i) const { return client[i].fh->get_handle (); }
+ bool is_connected (int i) const { return client[i].state == fc_connected; }
PUNICODE_STRING get_pipe_name ();
DWORD listen_client_thread ();
void fifo_client_lock () { _fifo_client_lock.lock (); }
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 59325860d..991494aa8 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -822,17 +822,148 @@ fhandler_pipe::select_except (select_stuff *ss)
return s;
}
+static int
+peek_fifo (select_record *s, bool from_select)
+{
+ if (cygheap->fdtab.not_open (s->fd))
+{
+ s->thread_errno = EBADF;
+ return -1;
+}
+
+ int gotone = 0;
+ fhandler_fifo *fh = (fhandler_fifo *) s->fh;
+
+ if (s->read_selected)
+{
+ if (s->read_ready)
+ {
+ select_printf ("%s, already ready for read", fh->get_name ());
+ gotone = 1;
+ goto out;
+ }
+
+ if (fh->get_readahead_valid ())
+ {
+ select_printf ("readahead");
+ gotone = s->read_ready = true;
+ goto out;
+ }
+
+ if (fh->hit_eof ())
+ {
+ select_printf ("read: %s, saw EOF", fh->get_name ());
+ gotone = s->read_ready = true;
+ if (s->except_selected)
+ gotone += s->except_ready = true;
+ goto out;
+ }
+
+ fh->fifo_client_lock ();
+ for (int i = 0; i < fh->get_nclients (); i++)
+ if (fh->is_connected (i))
+ {
+ int n = pipe_data_available (s->fd, fh, fh->get_handle (i),
+false);
+ if (n > 0)
+ {
+ select_printf ("read: %s, ready for read: avail %d, client %d",
+ fh->get_name (), n, i);
+ fh->fifo_client_unlock ();
+ gotone += s->read_ready = true;
+ goto out;
+ }
+ }
+ fh->fifo_client_unlock ();
+}
+out:
+ if (s->write_selected)
+{
+ gotone += s->write_ready
+ = pipe_data_available (s->fd, fh, fh->get_handle (), true);
+ select_printf ("write: %s, gotone %d", fh->get_name (), gotone);
+}
+ return gotone;
+}
+
+static int start_thread_fifo (select_record *me, select_stuff *stuff);
+
+static DWORD WINAPI
+thread_fifo (void *arg)
+{
+ select_fifo_info *pi = (select_fifo_info *) arg;
+ DWORD sleep_time = 0;
+ bool looping = true;
+
+ while (looping)
+{
+ for (select_record *s = pi->start; (s = s->next); )
+ if (s->startup == start_thread_fifo)
+ {
+ if (peek_fifo (s, true))
+ looping = false;
+ if (pi->stop_thread)
+ {
+ select_printf ("stopping");
+ looping = false;
+ break;
+ }
+ }
+ if (!looping)
+ break;
+ Sleep (sleep_time >> 3);
+ if (sleep_time < 80)
+ ++sleep_time;
+ if (pi->stop_thread)
+ break;
+}
+ return 0;
+}
+
+static int
+start_thread_fifo (select_record *me, select_stuff *stuff)
+{
+ select_fifo_info *pi = stuff->device_specific_fifo;
+ if (pi->start)
+me->h = *((select_fifo_info *) stuff->device_specific_fifo)->thread;
+ else
+{
+ pi->start = &stuff->start;
+ pi->stop_thread = false;
+ pi->thread = new cygthread (thread_fifo, pi, "fifosel");
+ me->h = *pi->thread;
+ if (!me->h)
+ return 0;
+}
+ return 1;
+}
+
+static void
+fifo_cleanup (select_record *, select_stuff *stuff)
+{
+ select_fifo_info *pi = (select_fifo_info *) stuff->device_specific_fifo;
+ if (!pi)
+return;
+ if (pi->thread)
+{
+ pi->stop_thread = true;
+ pi->thread->detach ();
+}
+ delete pi;
+ stuff->device_specific_fifo = NULL;
+}
+
select_record *
fhandler_fifo::select_read (select_stuff *ss)
{
- if (!ss->device_specific_pipe
- && (ss->device_specific_pipe = new sele
[PATCH fifo 5/8] Cygwin: FIFO: update clone and dup
Deal with all clients.
---
winsup/cygwin/fhandler.h | 8 +---
winsup/cygwin/fhandler_fifo.cc | 25 +
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index af5f500bf..0ebc44e0d 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1309,9 +1309,11 @@ public:
fhandler_fifo *clone (cygheap_types malloc_type = HEAP_FHANDLER)
{
void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_fifo));
-fhandler_fifo *fh = new (ptr) fhandler_fifo (ptr);
-copyto (fh);
-return fh;
+fhandler_fifo *fhf = new (ptr) fhandler_fifo (ptr);
+copyto (fhf);
+for (int i = 0; i < nclients; i++)
+ fhf->client[i].fh = client[i].fh->fhandler_base::clone ();
+return fhf;
}
};
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 1dcb3b3df..c295c2393 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -816,6 +816,31 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
__seterrno ();
return -1;
}
+ for (int i = 0; i < nclients; i++)
+{
+ if (!DuplicateHandle (GetCurrentProcess (), client[i].fh->get_handle (),
+ GetCurrentProcess (),
+ &fhf->client[i].fh->get_handle (),
+ 0, true, DUPLICATE_SAME_ACCESS)
+ || !DuplicateHandle (GetCurrentProcess (), client[i].connect_evt,
+ GetCurrentProcess (),
+ &fhf->client[i].connect_evt,
+ 0, true, DUPLICATE_SAME_ACCESS)
+ || !DuplicateHandle (GetCurrentProcess (), client[i].dummy_evt,
+ GetCurrentProcess (),
+ &fhf->client[i].dummy_evt,
+ 0, true, DUPLICATE_SAME_ACCESS))
+ {
+ CloseHandle (fhf->read_ready);
+ CloseHandle (fhf->write_ready);
+ fhf->close ();
+ __seterrno ();
+ return -1;
+ }
+}
+ fhf->listen_client_thr = NULL;
+ fhf->lct_termination_evt = NULL;
+ fhf->fifo_client_unlock ();
return 0;
}
--
2.17.0
[PATCH fifo 6/8] Cygwin: FIFO: update fixup_after_fork
Fixup each client. Reset listen_client_thr and lct_termination_evt.
---
winsup/cygwin/fhandler_fifo.cc | 9 +
1 file changed, 9 insertions(+)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index c295c2393..7a592aa0d 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -850,6 +850,15 @@ fhandler_fifo::fixup_after_fork (HANDLE parent)
fhandler_base::fixup_after_fork (parent);
fork_fixup (parent, read_ready, "read_ready");
fork_fixup (parent, write_ready, "write_ready");
+ for (int i = 0; i < nclients; i++)
+{
+ client[i].fh->fhandler_base::fixup_after_fork (parent);
+ fork_fixup (parent, client[i].connect_evt, "connect_evt");
+ fork_fixup (parent, client[i].dummy_evt, "dummy_evt");
+}
+ listen_client_thr = NULL;
+ lct_termination_evt = NULL;
+ fifo_client_unlock ();
}
void
--
2.17.0
[PATCH fifo 2/8] Cygwin: FIFO: allow multiple writers
Introduce a 'fifo_client_handler' structure that can be used by a
reader to communicate with a writer using an instance of the named
pipe. An fhandler_fifo opened for reading creates a thread that does
the following:
- maintains a list of fifo_client_handlers
- listens for_clients trying to connect
- creates new pipe instances as needed so that there's always at
least one available for connecting.
The pipe instances are initially created in blocking mode, but they
are set to be non-blocking after a connection is made.
fhandler_fifo::raw_read now loops through the connected clients and
reads from the first one that has data available.
New fhandler_fifo methods: add_client, listen_client,
listen_client_thread, check_listen_client_thread.
Replace the create_pipe method by create_pipe_instance, which allows
unlimited pipe instances.
New helper functions: create_event, set_pipe_non_blocking.
---
winsup/cygwin/fhandler.h | 31 ++-
winsup/cygwin/fhandler_fifo.cc | 368 ++---
2 files changed, 367 insertions(+), 32 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 57e97c277..e7c4af6a1 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1235,20 +1235,49 @@ public:
};
#define CYGWIN_FIFO_PIPE_NAME_LEN 47
+#define MAX_CLIENTS 64
+
+enum fifo_client_connect_state
+ {
+ fc_unknown,
+ fc_connecting,
+ fc_connected,
+ fc_invalid
+ };
+
+struct fifo_client_handler
+{
+ fhandler_base *fh;
+ fifo_client_connect_state state;
+ HANDLE connect_evt;
+ HANDLE dummy_evt;/* Never signaled. */
+ fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL),
+ dummy_evt (NULL) {}
+ int connect ();
+ int close ();
+};
class fhandler_fifo: public fhandler_base
{
HANDLE read_ready;
HANDLE write_ready;
+ HANDLE listen_client_thr;
+ HANDLE lct_termination_evt;
UNICODE_STRING pipe_name;
WCHAR pipe_name_buf[CYGWIN_FIFO_PIPE_NAME_LEN + 1];
+ fifo_client_handler client[MAX_CLIENTS];
+ int nclients, nconnected;
bool __reg2 wait (HANDLE);
NTSTATUS npfs_handle (HANDLE &);
- HANDLE create_pipe ();
+ HANDLE create_pipe_instance (bool);
NTSTATUS open_pipe ();
+ int disconnect_and_reconnect (int);
+ int add_client ();
+ bool listen_client ();
public:
fhandler_fifo ();
PUNICODE_STRING get_pipe_name ();
+ DWORD listen_client_thread ();
int open (int, mode_t);
off_t lseek (off_t offset, int whence);
int close ();
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index cb269e344..e91e88050 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -31,8 +31,9 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
|| _s == STATUS_PIPE_EMPTY; })
fhandler_fifo::fhandler_fifo ():
- fhandler_base (),
- read_ready (NULL), write_ready (NULL)
+ fhandler_base (), read_ready (NULL), write_ready (NULL),
+ listen_client_thr (NULL), lct_termination_evt (NULL), nclients (0),
+ nconnected (0)
{
pipe_name_buf[0] = L'\0';
need_fork_fixup (true);
@@ -78,6 +79,94 @@ fhandler_fifo::arm (HANDLE h)
return res;
}
+static HANDLE
+create_event ()
+{
+ NTSTATUS status;
+ OBJECT_ATTRIBUTES attr;
+ HANDLE evt = NULL;
+
+ InitializeObjectAttributes (&attr, NULL, 0, NULL, NULL);
+ status = NtCreateEvent (&evt, EVENT_ALL_ACCESS, &attr,
+ NotificationEvent, FALSE);
+ if (!NT_SUCCESS (status))
+__seterrno_from_nt_status (status);
+ return evt;
+}
+
+
+static void
+set_pipe_non_blocking (HANDLE ph, bool nonblocking)
+{
+ NTSTATUS status;
+ IO_STATUS_BLOCK io;
+ FILE_PIPE_INFORMATION fpi;
+
+ fpi.ReadMode = FILE_PIPE_MESSAGE_MODE;
+ fpi.CompletionMode = nonblocking ? FILE_PIPE_COMPLETE_OPERATION
+: FILE_PIPE_QUEUE_OPERATION;
+ status = NtSetInformationFile (ph, &io, &fpi, sizeof fpi,
+FilePipeInformation);
+ if (!NT_SUCCESS (status))
+debug_printf ("NtSetInformationFile(FilePipeInformation): %y", status);
+}
+
+/* The pipe instance is always in blocking mode when this is called. */
+int
+fifo_client_handler::connect ()
+{
+ NTSTATUS status;
+ IO_STATUS_BLOCK io;
+
+ if (connect_evt)
+ResetEvent (connect_evt);
+ else if (!(connect_evt = create_event ()))
+return -1;
+ status = NtFsControlFile (fh->get_handle (), connect_evt, NULL, NULL, &io,
+ FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
+ switch (status)
+{
+case STATUS_PENDING:
+case STATUS_PIPE_LISTENING:
+ state = fc_connecting;
+ break;
+case STATUS_PIPE_CONNECTED:
+ state = fc_connected;
+ set_pipe_non_blocking (fh->get_handle (), true);
+ break;
+default:
+ __seterrno_from_nt_status (status);
+ return -1;
+}
+ return 0;
+}
+
+int
+fhandler_fifo::disconnect_and_reconnect (int i)
+{
+ NTSTATUS status;
+ IO_STATUS_BLOCK io;
+ HANDLE
[PATCH fifo 7/8] Cygwin: FIFO: update set_close_on_exec
Deal with each client.
---
winsup/cygwin/fhandler_fifo.cc | 6 ++
1 file changed, 6 insertions(+)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 7a592aa0d..2c20444c6 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -867,4 +867,10 @@ fhandler_fifo::set_close_on_exec (bool val)
fhandler_base::set_close_on_exec (val);
set_no_inheritance (read_ready, val);
set_no_inheritance (write_ready, val);
+ for (int i = 0; i < nclients; i++)
+{
+ client[i].fh->fhandler_base::set_close_on_exec (val);
+ set_no_inheritance (client[i].connect_evt, val);
+ set_no_inheritance (client[i].dummy_evt, val);
+}
}
--
2.17.0
[PATCH fifo 4/8] Cygwin: FIFO: improve EOF detection
Add a hit_eof method that tries to detect whether any clients are
connected. Before concluding that there are none, it gives the
listen_client thread time to update the client data.
---
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_fifo.cc | 21 -
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 997dc0b6d..af5f500bf 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1277,6 +1277,7 @@ class fhandler_fifo: public fhandler_base
bool listen_client ();
public:
fhandler_fifo ();
+ bool hit_eof ();
PUNICODE_STRING get_pipe_name ();
DWORD listen_client_thread ();
void fifo_client_lock () { _fifo_client_lock.lock (); }
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index b0016ee90..1dcb3b3df 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -654,6 +654,25 @@ fhandler_fifo::raw_write (const void *ptr, size_t len)
return ret;
}
+/* A FIFO open for reading is at EOF if no process has it open for
+ writing. We test this by checking nconnected. But we must take
+ account of the possible delay from the time of connection to the
+ time the connection is recorded by the listen_client thread. */
+bool
+fhandler_fifo::hit_eof ()
+{
+ fifo_client_lock ();
+ bool eof = (nconnected == 0);
+ fifo_client_unlock ();
+ if (eof)
+{
+ /* Give the listen_client thread time to catch up, then recheck. */
+ Sleep (1);
+ eof = (nconnected == 0);
+}
+ return eof;
+}
+
void __reg3
fhandler_fifo::raw_read (void *in_ptr, size_t& len)
{
@@ -665,7 +684,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
while (1)
{
- if (nconnected == 0) /* EOF */
+ if (hit_eof ())
{
len = 0;
return;
--
2.17.0
Re: [PATCH fifo 0/8] Allow a FIFO to have multiple writers
On 3/23/2019 4:02 PM, Corinna Vinschen wrote: > Your patch series looks really good. For now I pushed it into the > topic/fifo branch as you suggested. Thanks. > Just be aware that it won't get much 3rd party testing this way, so as > soon as you feel more confident, let's move it into master. At that > time I'll create a Cygwin 3.0.y branch for bugfix releases, but the > snapshots will continue to be created from master so we get at least > *some* folks testing it. Sounds good. It shouldn't be too long. > A big thank you already! > > Oh, there's obviously a downside to this, too: You'll have to create > a release note for this change at one point ;) Oh yeah, I forgot about that. No problem. Ken
[PATCH fifo 0/2] Add support for duplex FIFOs
The second patch in this series enables opening a FIFO with O_RDWR access. The underlying Windows named pipe is creted with duplex access, and its handle is made the I/O handle of the first client. While testing this, I had some mysterious crashes, which are fixed by the first patch. I tested the patch in two ways. First, I went back to Kerrisk's server/client programs cited in https://cygwin.com/ml/cygwin/2015-03/msg00047.html. The server program opens a FIFO twice, once with O_RDONLY access and once with O_WRONLY access. The fd from the second call is never used; the purpose of that call is simply to make sure that attempts to read from the FIFO will never indicate EOF. I removed the second call and instead opened the FIFO only once, with O_RDWR access. The server/client programs still worked as expected. The second test was the following sequence of commands in a bash shell: $ mkfifo foo $ exec 7<>foo $ echo blah > foo $ read bar <&7 $ echo $bar blah Ken Ken Brown (2): Cygwin: FIFO: avoid crashes when cloning a client Cygwin: FIFO: add support for the duplex case winsup/cygwin/fhandler.h | 7 ++- winsup/cygwin/fhandler_fifo.cc | 79 +- 2 files changed, 74 insertions(+), 12 deletions(-) -- 2.17.0
[PATCH fifo 1/2] Cygwin: FIFO: avoid crashes when cloning a client
fhandler_fifo::clone called fhandler_base::clone on each client fhandler. But those fhandlers are actually fhandler_fifo objects, so when fhandler_base::clone calls copyto, it's actually fhandler_fifo::copyto that gets called. This can lead to mysterious crashes. Fix this by simply calling clone (which translates to fhandler_fifo::clone) on each client fhandler. --- winsup/cygwin/fhandler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index f6982f0ba..ef34f9c40 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -1316,7 +1316,7 @@ public: fhandler_fifo *fhf = new (ptr) fhandler_fifo (ptr); copyto (fhf); for (int i = 0; i < nclients; i++) - fhf->client[i].fh = client[i].fh->fhandler_base::clone (); + fhf->client[i].fh = client[i].fh->clone (); return fhf; } }; -- 2.17.0
[PATCH fifo 2/2] Cygwin: FIFO: add support for the duplex case
If a FIFO is opened with O_RDWR access, create the pipe with
read/write access, and make the first client have the handle of that
pipe as its I/O handle.
Adjust fhandler_fifo::raw_read to account for the result of trying to
read from that client if there's no data.
---
winsup/cygwin/fhandler.h | 5 +++
winsup/cygwin/fhandler_fifo.cc | 79 +-
2 files changed, 73 insertions(+), 11 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index ef34f9c40..3398cc625 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1253,6 +1253,10 @@ struct fifo_client_handler
HANDLE dummy_evt;/* Never signaled. */
fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL),
dummy_evt (NULL) {}
+ fifo_client_handler (fhandler_base *_fh, fifo_client_connect_state _state,
+ HANDLE _connect_evt, HANDLE _dummy_evt)
+: fh (_fh), state (_state), connect_evt (_connect_evt),
+ dummy_evt (_dummy_evt) {}
int connect ();
int close ();
};
@@ -1268,6 +1272,7 @@ class fhandler_fifo: public fhandler_base
fifo_client_handler client[MAX_CLIENTS];
int nclients, nconnected;
af_unix_spinlock_t _fifo_client_lock;
+ bool _duplexer;
bool __reg2 wait (HANDLE);
NTSTATUS npfs_handle (HANDLE &);
HANDLE create_pipe_instance (bool);
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 2c20444c6..7847cca82 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -33,7 +33,7 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
fhandler_fifo::fhandler_fifo ():
fhandler_base (), read_ready (NULL), write_ready (NULL),
listen_client_thr (NULL), lct_termination_evt (NULL), nclients (0),
- nconnected (0)
+ nconnected (0), _duplexer (false)
{
pipe_name_buf[0] = L'\0';
need_fork_fixup (true);
@@ -224,6 +224,8 @@ fhandler_fifo::create_pipe_instance (bool first)
}
access = GENERIC_READ | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
| SYNCHRONIZE;
+ if (first && _duplexer)
+access |= GENERIC_WRITE;
sharing = FILE_SHARE_READ | FILE_SHARE_WRITE;
hattr = OBJ_INHERIT;
if (first)
@@ -437,7 +439,7 @@ fhandler_fifo::open (int flags, mode_t)
case O_RDWR:
reader = true;
writer = false;
- duplexer = true;
+ duplexer = _duplexer = true;
break;
default:
set_errno (EINVAL);
@@ -447,7 +449,7 @@ fhandler_fifo::open (int flags, mode_t)
debug_only_printf ("reader %d, writer %d, duplexer %d", reader, writer,
duplexer);
set_flags (flags);
- if (reader)
+ if (reader && !duplexer)
nohandle (true);
/* Create control events for this named pipe */
@@ -472,6 +474,48 @@ fhandler_fifo::open (int flags, mode_t)
goto out;
}
+ /* If we're a duplexer, create the pipe and the first client. */
+ if (duplexer)
+{
+ HANDLE ph, connect_evt, dummy_evt;
+ fhandler_base *fh;
+
+ ph = create_pipe_instance (true);
+ if (!ph)
+ {
+ res = error_errno_set;
+ goto out;
+ }
+ set_io_handle (ph);
+ set_pipe_non_blocking (ph, true);
+ if (!(fh = build_fh_dev (dev (
+ {
+ set_errno (EMFILE);
+ res = error_errno_set;
+ goto out;
+ }
+ fh->set_io_handle (ph);
+ fh->set_flags (flags);
+ if (!(connect_evt = create_event ()))
+ {
+ res = error_errno_set;
+ fh->close ();
+ delete fh;
+ goto out;
+ }
+ if (!(dummy_evt = create_event ()))
+ {
+ res = error_errno_set;
+ delete fh;
+ fh->close ();
+ CloseHandle (connect_evt);
+ goto out;
+ }
+ client[0] = fifo_client_handler (fh, fc_connected, connect_evt,
+ dummy_evt);
+ nconnected = nclients = 1;
+}
+
/* If we're reading, start the listen_client thread (which should
signal read_ready), and wait for a writer. */
if (reader)
@@ -482,8 +526,8 @@ fhandler_fifo::open (int flags, mode_t)
res = error_errno_set;
goto out;
}
- /* Wait for the listen_client thread to create the pipe and
-signal read_ready. This should be quick. */
+ /* Wait for the listen_client thread to signal read_ready. This
+should be quick. */
HANDLE w[2] = { listen_client_thr, read_ready };
switch (WaitForMultipleObjects (2, w, FALSE, INFINITE))
{
@@ -703,12 +747,25 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
fifo_client_unlock ();
return;
}
- else if (nread < 0 && GetLastError () != ERROR_NO_DATA)
- {
- fifo_client_unlock ();
- goto errout;
- }
- else if (nread == 0) /* Client has disconnected. */
+ /* In the duplex case w
Re: [PATCH fifo 0/2] Add support for duplex FIFOs
Hi Corinna,
On 3/26/2019 4:36 AM, Corinna Vinschen wrote:
> Hi Ken,
>
> On Mar 25 23:06, Ken Brown wrote:
>> The second patch in this series enables opening a FIFO with O_RDWR
>> access. The underlying Windows named pipe is creted with duplex
>> access, and its handle is made the I/O handle of the first client.
>>
>> While testing this, I had some mysterious crashes, which are fixed by
>> the first patch.
>
> I rebased the topic/fifo branch on top of master and force-pushed with
> your patches. Make sure to reset your working tree to origin/topic/fifo
> and add any further patches on top.
I'm comfortable now with merging topic/fifo into master. I've tested the new
select and fork code [*], and they seem to work as expected. That was the last
thing holding me up.
As soon as the merge is done, I'll send a patch with release notes.
Ken
[*] For the record, I'm attaching the two test programs I used. In each case I
ran the program in one terminal and typed "echo blah > /tmp/myfifo" in a second
terminal.
#include
#include
#include
#include
#include
#include
#include
int
main ()
{
int fd;
ssize_t nbytes;
char buf[5];
if (mkfifo ("/tmp/myfifo", S_IRUSR | S_IWUSR | S_IWGRP) < 0
&& errno != EEXIST)
{
perror ("mkfifo");
exit (-1);
}
if ((fd = open ("/tmp/myfifo", O_RDWR)) < 0)
{
perror ("open");
exit (-1);
}
switch (fork ())
{
case -1:
perror ("fork");
exit (-1);
case 0: /* Child. */
nbytes = read (fd, buf, 4);
if (nbytes != 4)
{
perror ("read");
exit (-1);
}
buf[4] = '\0';
printf ("child read %d bytes: %s\n", nbytes, buf);
if (close (fd) < 0)
{
perror ("child close");
exit (-1);
}
exit (0);
default:/* Parent. */
if (close (fd) < 0)
{
perror ("parent close");
exit (-1);
}
printf ("parent waiting for child to read\n");
wait (NULL);
break;
}
}
#include
#include
#include
#include
#include
#include
#include
int
main ()
{
int fd, nfds;
fd_set readfds;
ssize_t nbytes;
char buf[5];
if (mkfifo ("/tmp/myfifo", S_IRUSR | S_IWUSR | S_IWGRP) < 0
&& errno != EEXIST)
{
perror ("mkfifo");
exit (-1);
}
if ((fd = open ("/tmp/myfifo", O_RDONLY)) < 0)
{
perror ("open");
exit (-1);
}
FD_ZERO (&readfds);
FD_SET (fd, &readfds);
if ((nfds = select (fd + 1, &readfds, NULL, NULL, NULL)) < 0)
{
perror ("select");
exit (-1);
}
if (FD_ISSET (fd, &readfds))
printf ("/tmp/myfifo is ready for reading\n");
else
printf ("something's wrong\n");
nbytes = read (fd, buf, 4);
if (nbytes != 4)
{
perror ("read");
exit (-1);
}
buf[4] = '\0';
printf ("read %d bytes: %s\n", nbytes, buf);
if (close (fd) < 0)
{
perror ("close");
exit (-1);
}
}
[PATCH] Cygwin: document the recent FIFO changes
--- winsup/cygwin/release/3.1.0 | 14 ++ winsup/doc/new-features.xml | 12 2 files changed, 26 insertions(+) create mode 100644 winsup/cygwin/release/3.1.0 diff --git a/winsup/cygwin/release/3.1.0 b/winsup/cygwin/release/3.1.0 new file mode 100644 index 0..1f017bfd1 --- /dev/null +++ b/winsup/cygwin/release/3.1.0 @@ -0,0 +1,14 @@ +What's new: +--- + + +What changed: +- + +- FIFOs can now be opened multiple times for writing. + Addresses: https://cygwin.com/ml/cygwin/2015-03/msg00047.html + https://cygwin.com/ml/cygwin/2015-12/msg00311.html + + +Bug Fixes +- diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml index e14fbb1e8..c87601e9d 100644 --- a/winsup/doc/new-features.xml +++ b/winsup/doc/new-features.xml @@ -4,6 +4,18 @@ What's new and what changed in Cygwin +What's new and what changed in 3.1 + + + + +FIFOs can now be opened multiple times for writing. + + + + + + What's new and what changed in 3.0 -- 2.17.0
[PATCH] Cygwin: FIFO: implement clear_readahead
Make fhandler_base::clear_readahead virtual, and implement
fhandler_fifo::clear_readahead. This is called by
dtable::fixup_after_exec; it clears the readahead in each client.
---
winsup/cygwin/fhandler.h | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 3398cc625..21fec9e38 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -444,7 +444,7 @@ public:
return dev ().native ();
}
virtual bg_check_types bg_check (int, bool = false) {return bg_ok;}
- void clear_readahead ()
+ virtual void clear_readahead ()
{
raixput = raixget = ralen = rabuflen = 0;
rabuf = NULL;
@@ -1302,6 +1302,12 @@ public:
bool arm (HANDLE h);
void fixup_after_fork (HANDLE);
int __reg2 fstatvfs (struct statvfs *buf);
+ void clear_readahead ()
+ {
+fhandler_base::clear_readahead ();
+for (int i = 0; i < nclients; i++)
+ client[i].fh->clear_readahead ();
+ }
select_record *select_read (select_stuff *);
select_record *select_write (select_stuff *);
select_record *select_except (select_stuff *);
--
2.17.0
Re: [PATCH fifo 0/2] Add support for duplex FIFOs
On 3/28/2019 4:13 PM, Corinna Vinschen wrote: > On Mar 28 19:01, Achim Gratz wrote: >> Corinna Vinschen writes: >>> Done. I also pushed out new dev snapshots. >> >> No good deed goes unpunished… >> >> Whith the 20190327 snapshot our main data processing application is >> broken. It looks like it should almost work, it doesn't crash or >> anything, but the pipe that delivers a script+data into gnuplot seems to >> either skip or overwrite data and then gnuplot bails with a syntax >> error. Depending on exactly which data I try to plot I get the first or >> first few plots out through the whole processing pipe (that ends in a >> PDF file), albeit sometimes with incomplete data. Doing each of the >> steps manually (i.e. writing the gnuplot script into a file, then feed >> that into gnuplot, then the output from gnuplot inth ghostscript) does >> work correctly. I have not yet been able to reduce this down to some >> simpler test case, so I had to roll back to the previous snapshot. I >> still have it installed on the development system, though. > > I'm pretty sure Ken would be happy about an STC. Yes, please. Barring that, is there any chance I could see the relevant code, or at least enough of it so that I can see how FIFOs are being used? Thanks, and thanks for testing. Ken
Re: [PATCH fifo 0/2] Add support for duplex FIFOs
On 3/29/2019 3:17 AM, Achim Gratz wrote: > Ken Brown writes: >>> I'm pretty sure Ken would be happy about an STC. >> >> Yes, please. Barring that, is there any chance I could see the relevant >> code, >> or at least enough of it so that I can see how FIFOs are being used? > > Well, I'm trying -- but got nothing so far. As the individual FIFO seem > to work and the error happens pretty early, I think it has something to > do with either switching between different FIFO (which the original code > does) or some race between fill and drain, possibly around a buffer > boundary (in my limited testing it always seemed to happen in the same > place for the same data). I found a bug in my fhandler_fifo::raw_write code that could explain the problem. The call to NtWriteFile in that function always returns immediately because the Windows named pipe underlying the FIFO is non-blocking. If it can't write because the pipe buffer is full, raw_write returns -1 with error EAGAIN. That's wrong if the FIFO was opened in blocking mode. I'll have to think about how to best handle this. I think I might be able to imitate what's done in fhandler_socket_unix::sendmsg in the topic/af_unix branch. Ken
Re: [PATCH fifo 2/2] Cygwin: FIFO: add support for the duplex case
Hi Takashi, On 3/30/2019 9:30 AM, Takashi Yano wrote: > Hi Ken, > > Do these patches enable to open FIFO multiple times with O_RDWR? > mc (midnight commander) tries to open FIFO twice with O_RDWR if > SHELL=/bin/tcsh, but fails. > https://cygwin.com/ml/cygwin/2017-03/msg00188.html No, I'm sorry. At the moment, a FIFO can only be opened once for reading. I plan to work on this next, but first I have some bugs to fix. Ken
Re: [PATCH fifo 0/2] Add support for duplex FIFOs
On 4/3/2019 8:33 AM, Achim Gratz wrote: > Achim Gratz writes: >> OK, a bit more info: The whole thing runs from a perl script (actually a >> module) that opens pipes to gnuplot and ghostscript. This code is >> _really_ old and has seen a lot of Cygwin releases, so it has options to >> either use temporary files, named pipes aka FIFO or direct pipes. Using >> temporary files serializes the execution and using a pipe chain is >> _really_ slow (like a hundred times, which is mostly tied up in system >> for a reason that I don't understand), so using FIFO is the default. >> Your new FIFO code increases the system time by about a factor of 10 in >> my tests, btw. > > So I've finalöly got around to fixing the pipe performance problem by > fooling the programs involved to think they are using files: have them > reading from /proc/self/fd/0 and writing to /proc/self/fd/1 gives me the > same performance as using a named FIFO. > > Incidentally, that workaround still works when I switch to the 20190402 > snapshot, while named FIFO fails as with the older snapshot (as > expected), so that seems to take a different code path. Maybe that > helps in finding the problem? Thanks. In the meantime, I've fixed the raw_write bug that I mentioned in an earlier post, along with some other bugs I found. I want to do a little more testing before I send the patches. Ken
[PATCH 03/14] Cygwin: FIFO: remember the type of the fhandler
Add data members 'reader', 'writer', and 'duplexer' to the
fhandler_fifo class. Set them in fhandler_fifo::open. ('duplexer'
replaces the previous '_duplexer'.)
This will be useful in later commits.
---
winsup/cygwin/fhandler.h | 2 +-
winsup/cygwin/fhandler_fifo.cc | 14 --
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 2898db4df..59d9dad16 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1272,7 +1272,7 @@ class fhandler_fifo: public fhandler_base
fifo_client_handler fc_handler[MAX_CLIENTS];
int nhandlers, nconnected;
af_unix_spinlock_t _fifo_client_lock;
- bool _duplexer;
+ bool reader, writer, duplexer;
bool __reg2 wait (HANDLE);
NTSTATUS npfs_handle (HANDLE &);
HANDLE create_pipe_instance (bool);
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 70551ebac..258c3cf8a 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -33,7 +33,7 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
fhandler_fifo::fhandler_fifo ():
fhandler_base (), read_ready (NULL), write_ready (NULL),
listen_client_thr (NULL), lct_termination_evt (NULL), nhandlers (0),
- nconnected (0), _duplexer (false)
+ nconnected (0), reader (false), writer (false), duplexer (false)
{
pipe_name_buf[0] = L'\0';
need_fork_fixup (true);
@@ -224,7 +224,7 @@ fhandler_fifo::create_pipe_instance (bool first)
}
access = GENERIC_READ | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
| SYNCHRONIZE;
- if (first && _duplexer)
+ if (first && duplexer)
access |= GENERIC_WRITE;
sharing = FILE_SHARE_READ | FILE_SHARE_WRITE;
hattr = OBJ_INHERIT;
@@ -421,25 +421,19 @@ fhandler_fifo::open (int flags, mode_t)
error_errno_set,
error_set_errno
} res;
- bool reader, writer, duplexer;
/* Determine what we're doing with this fhandler: reading, writing, both */
switch (flags & O_ACCMODE)
{
case O_RDONLY:
reader = true;
- writer = false;
- duplexer = false;
break;
case O_WRONLY:
writer = true;
- reader = false;
- duplexer = false;
break;
case O_RDWR:
reader = true;
- writer = false;
- duplexer = _duplexer = true;
+ duplexer = true;
break;
default:
set_errno (EINVAL);
@@ -769,7 +763,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
fifo_client_unlock ();
goto errout;
}
- else if (nread == 0 && (!_duplexer || i > 0))
+ else if (nread == 0 && (!duplexer || i > 0))
/* Client has disconnected. */
{
fc_handler[i].state = fc_invalid;
--
2.17.0
[PATCH 00/14] FIFO bug fixes and code simplifications
Ken Brown (14): Cygwin: FIFO: rename client[] to fc_handler[] Cygwin: FIFO: hit_eof: add a call to fifo_client_lock Cygwin: FIFO: remember the type of the fhandler Cygwin: FIFO: fix a thinko in listen_client_thread Cygwin: FIFO: fix the error checking in raw_read Cygwin: check for STATUS_PENDING in fhandler_base::raw_read Cygwin: FIFO: code simplification: don't overload get_handle Cygwin: FIFO: fix fifo_client_handler::close Cygwin: FIFO: fix the use of the read_ready event Cygwin: FIFO: use a retry loop when opening a writer Cygwin: FIFO: fix clone Cygwin: FIFO: start the listen_client thread when duping a reader Cygwin: FIFO: improve raw_write Cygwin: FIFO: fix and simplify listen_client_thread winsup/cygwin/fhandler.cc | 14 +- winsup/cygwin/fhandler.h | 43 +-- winsup/cygwin/fhandler_fifo.cc | 580 + winsup/cygwin/select.cc| 4 +- 4 files changed, 342 insertions(+), 299 deletions(-) -- 2.17.0
[PATCH 04/14] Cygwin: FIFO: fix a thinko in listen_client_thread
--- winsup/cygwin/fhandler_fifo.cc | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc index 258c3cf8a..764420ffd 100644 --- a/winsup/cygwin/fhandler_fifo.cc +++ b/winsup/cygwin/fhandler_fifo.cc @@ -362,7 +362,10 @@ fhandler_fifo::listen_client_thread () fifo_client_unlock (); goto errout; } - /* Fall through. */ + else + /* Recheck fc_handler[i].state. */ + i--; + break; case fc_connected: w[i] = fc_handler[i].dummy_evt; break; -- 2.17.0
[PATCH 11/14] Cygwin: FIFO: fix clone
After copyto is called, make the new fhandler's pipe_name point to the
new fhandler's pipe_name_buf, which is a *copy* of the old fhandler's
pipe_name_buf. Previously, get_pipe_name would return the wrong
result after a clone/dup, causing create_pipe_instance and open_pipe
to fail.
Also, stop the listen_client thread when cloning. Otherwise the
thread can keep accepting connections that the cloned fhandler won't
know about.
Do this via a new method fhandler_fifo::stop_listen_client, extracted
from fhandler_fifo::close.
---
winsup/cygwin/fhandler.h | 6 ++
winsup/cygwin/fhandler_fifo.cc | 20 +++-
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 1e26c651f..aea02c2b3 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1280,6 +1280,7 @@ class fhandler_fifo: public fhandler_base
int disconnect_and_reconnect (int);
int add_client_handler ();
bool listen_client ();
+ int stop_listen_client ();
public:
fhandler_fifo ();
bool hit_eof ();
@@ -1326,7 +1327,12 @@ public:
{
void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_fifo));
fhandler_fifo *fhf = new (ptr) fhandler_fifo (ptr);
+/* We don't want our client list to change any more. */
+stop_listen_client ();
copyto (fhf);
+/* fhf->pipe_name_buf is a *copy* of this->pipe_name_buf, but
+ fhf->pipe_name.Buffer == this->pipe_name_buf. */
+fhf->pipe_name.Buffer = fhf->pipe_name_buf;
for (int i = 0; i < nhandlers; i++)
fhf->fc_handler[i].fh = fc_handler[i].fh->clone ();
return fhf;
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index d4d2b3883..a3ecbeb4a 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -812,9 +812,9 @@ fifo_client_handler::close ()
}
int
-fhandler_fifo::close ()
+fhandler_fifo::stop_listen_client ()
{
- int res = 0;
+ int ret = 0;
HANDLE evt = InterlockedExchangePointer (&lct_termination_evt, NULL);
HANDLE thr = InterlockedExchangePointer (&listen_client_thr, NULL);
if (thr)
@@ -825,19 +825,29 @@ fhandler_fifo::close ()
DWORD err;
GetExitCodeThread (thr, &err);
if (err)
- debug_printf ("listen_client_thread exited with code %d", err);
+ {
+ ret = -1;
+ debug_printf ("listen_client_thread exited with error, %E");
+ }
CloseHandle (thr);
}
if (evt)
CloseHandle (evt);
+ return ret;
+}
+
+int
+fhandler_fifo::close ()
+{
+ int ret = stop_listen_client ();
if (read_ready)
CloseHandle (read_ready);
if (write_ready)
CloseHandle (write_ready);
for (int i = 0; i < nhandlers; i++)
if (fc_handler[i].close () < 0)
- res = -1;
- return fhandler_base::close () || res;
+ ret = -1;
+ return fhandler_base::close () || ret;
}
int
--
2.17.0
[PATCH 13/14] Cygwin: FIFO: improve raw_write
Don't set the write end of the pipe to non-blocking mode if the FIFO
is opened in blocking mode.
In fhandler_fifo::raw_write in blocking mode, wait for the write to
complete rather than returning -1 with EAGAIN.
If the amount to write is large, write in smaller chunks (of size
determined by a new data member max_atomic_write), as in
fhandler_base_overlapped.
---
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_fifo.cc | 96 +++---
2 files changed, 79 insertions(+), 18 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index aea02c2b3..fd205a6db 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1273,6 +1273,7 @@ class fhandler_fifo: public fhandler_base
int nhandlers, nconnected;
af_unix_spinlock_t _fifo_client_lock;
bool reader, writer, duplexer;
+ size_t max_atomic_write;
bool __reg2 wait (HANDLE);
NTSTATUS npfs_handle (HANDLE &);
HANDLE create_pipe_instance (bool);
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index fe4c67bdf..ce078d74d 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -22,6 +22,9 @@
#include "ntdll.h"
#include "cygwait.h"
+#define STATUS_THREAD_SIGNALED ((NTSTATUS)0xE001)
+#define STATUS_THREAD_CANCELED ((NTSTATUS)0xE002)
+
/* This is only to be used for writers. When reading,
STATUS_PIPE_EMPTY simply means there's no data to be read. */
#define STATUS_PIPE_IS_CLOSED(status) \
@@ -39,7 +42,8 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
fhandler_fifo::fhandler_fifo ():
fhandler_base (), read_ready (NULL), write_ready (NULL),
listen_client_thr (NULL), lct_termination_evt (NULL), nhandlers (0),
- nconnected (0), reader (false), writer (false), duplexer (false)
+ nconnected (0), reader (false), writer (false), duplexer (false),
+ max_atomic_write (DEFAULT_PIPEBUFSIZE)
{
pipe_name_buf[0] = L'\0';
need_fork_fixup (true);
@@ -559,7 +563,7 @@ fhandler_fifo::open (int flags, mode_t)
NTSTATUS status = open_pipe ();
if (NT_SUCCESS (status))
{
- set_pipe_non_blocking (get_handle (), true);
+ set_pipe_non_blocking (get_handle (), flags & O_NONBLOCK);
if (!arm (write_ready))
res = error_set_errno;
else
@@ -661,28 +665,84 @@ ssize_t __reg3
fhandler_fifo::raw_write (const void *ptr, size_t len)
{
ssize_t ret = -1;
+ size_t nbytes = 0, chunk;
NTSTATUS status;
IO_STATUS_BLOCK io;
+ HANDLE evt = NULL;
- status = NtWriteFile (get_handle (), NULL, NULL, NULL, &io,
- (PVOID) ptr, len, NULL, NULL);
- if (NT_SUCCESS (status))
+ if (len <= max_atomic_write)
+chunk = len;
+ else if (is_nonblocking ())
+chunk = len = max_atomic_write;
+ else
+chunk = max_atomic_write;
+
+ /* Create a wait event if the FIFO is in blocking mode. */
+ if (!is_nonblocking () && !(evt = CreateEvent (NULL, false, false, NULL)))
+return -1;
+
+ /* Write in chunks, accumulating a total. If there's an error, just
+ return the accumulated total unless the first write fails, in
+ which case return -1. */
+ while (nbytes < len)
{
- /* NtWriteFile returns success with # of bytes written == 0 in
-case writing on a non-blocking pipe fails if the pipe buffer
-is full. */
- if (io.Information == 0)
- set_errno (EAGAIN);
+ ULONG_PTR nbytes_now = 0;
+ size_t left = len - nbytes;
+ size_t len1;
+ if (left > chunk)
+ len1 = chunk;
else
- ret = io.Information;
-}
- else if (STATUS_PIPE_IS_CLOSED (status))
-{
- set_errno (EPIPE);
- raise (SIGPIPE);
+ len1 = left;
+ nbytes_now = 0;
+ status = NtWriteFile (get_handle (), evt, NULL, NULL, &io,
+ (PVOID) ptr, len1, NULL, NULL);
+ if (evt && status == STATUS_PENDING)
+ {
+ DWORD waitret = cygwait (evt, cw_infinite, cw_cancel | cw_sig_eintr);
+ switch (waitret)
+ {
+ case WAIT_OBJECT_0:
+ status = io.Status;
+ break;
+ case WAIT_SIGNALED:
+ status = STATUS_THREAD_SIGNALED;
+ break;
+ case WAIT_CANCELED:
+ status = STATUS_THREAD_CANCELED;
+ break;
+ default:
+ break;
+ }
+ }
+ if (NT_SUCCESS (status))
+ {
+ nbytes_now = io.Information;
+ /* NtWriteFile returns success with # of bytes written == 0
+if writing on a non-blocking pipe fails because the pipe
+buffer doesn't have sufficient space. */
+ if (nbytes_now == 0)
+ set_errno (EAGAIN);
+ ptr = ((char *) ptr) + chunk;
+ nbytes += nbytes_now;
+ }
+ else if (STATUS_PIPE_IS_CLOSED (status))
+ {
+ set_errno (EPIPE);
+ raise (SIGPIPE);
+
[PATCH 07/14] Cygwin: FIFO: code simplification: don't overload get_handle
Rename fhandler_fifo::get_handle(int) to get_fc_handle(int), and
remove fhandler_fifo::get_handle(void).
---
winsup/cygwin/fhandler.h | 7 ---
winsup/cygwin/select.cc | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 59d9dad16..1e26c651f 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1284,9 +1284,10 @@ public:
fhandler_fifo ();
bool hit_eof ();
int get_nhandlers () const { return nhandlers; }
- HANDLE& get_handle () { return fhandler_base::get_handle (); }
- HANDLE get_handle (int i) const { return fc_handler[i].fh->get_handle (); }
- bool is_connected (int i) const { return fc_handler[i].state ==
fc_connected; }
+ HANDLE get_fc_handle (int i) const
+ { return fc_handler[i].fh->get_handle (); }
+ bool is_connected (int i) const
+ { return fc_handler[i].state == fc_connected; }
PUNICODE_STRING get_pipe_name ();
DWORD listen_client_thread ();
void fifo_client_lock () { _fifo_client_lock.lock (); }
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index d257cc4ed..9cf892801 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -875,7 +875,7 @@ peek_fifo (select_record *s, bool from_select)
for (int i = 0; i < fh->get_nhandlers (); i++)
if (fh->is_connected (i))
{
- int n = pipe_data_available (s->fd, fh, fh->get_handle (i),
+ int n = pipe_data_available (s->fd, fh, fh->get_fc_handle (i),
false);
if (n > 0)
{
--
2.17.0
[PATCH 06/14] Cygwin: check for STATUS_PENDING in fhandler_base::raw_read
If NtReadFile returns STATUS_PENDING, wait for the read to complete.
This can happen, for instance, in the case of a FIFO opened with
O_RDRW.
---
winsup/cygwin/fhandler.cc | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index b0c9c50c3..a0c3dcce2 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -215,11 +215,23 @@ fhandler_base::raw_read (void *ptr, size_t& len)
NTSTATUS status;
IO_STATUS_BLOCK io;
int try_noreserve = 1;
+ DWORD waitret = WAIT_OBJECT_0;
retry:
status = NtReadFile (get_handle (), NULL, NULL, NULL, &io, ptr, len,
NULL, NULL);
- if (NT_SUCCESS (status))
+ if (status == STATUS_PENDING)
+{
+ waitret = cygwait (get_handle (), cw_infinite,
+cw_cancel | cw_sig_eintr);
+ if (waitret == WAIT_OBJECT_0)
+ status = io.Status;
+}
+ if (waitret == WAIT_CANCELED)
+pthread::static_cancel_self ();
+ else if (waitret == WAIT_SIGNALED)
+set_errno (EINTR);
+ else if (NT_SUCCESS (status))
len = io.Information;
else
{
--
2.17.0
[PATCH 02/14] Cygwin: FIFO: hit_eof: add a call to fifo_client_lock
The second check of nconnected needs to be protected by a lock as well
as the first.
---
winsup/cygwin/fhandler_fifo.cc | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 901696444..70551ebac 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -705,15 +705,20 @@ fhandler_fifo::raw_write (const void *ptr, size_t len)
bool
fhandler_fifo::hit_eof ()
{
- fifo_client_lock ();
- bool eof = (nconnected == 0);
- fifo_client_unlock ();
- if (eof)
-{
- /* Give the listen_client thread time to catch up, then recheck. */
- Sleep (1);
+ bool eof;
+ bool retry = true;
+
+retry:
+ fifo_client_lock ();
eof = (nconnected == 0);
-}
+ fifo_client_unlock ();
+ if (eof && retry)
+ {
+ retry = false;
+ /* Give the listen_client thread time to catch up. */
+ Sleep (1);
+ goto retry;
+ }
return eof;
}
--
2.17.0
[PATCH 10/14] Cygwin: FIFO: use a retry loop when opening a writer
There may be short periods when there's no pipe instance available.
Keep trying.
---
winsup/cygwin/fhandler_fifo.cc | 52 --
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 479021e8e..d4d2b3883 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -30,6 +30,12 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
|| _s == STATUS_PIPE_BROKEN \
|| _s == STATUS_PIPE_EMPTY; })
+#define STATUS_PIPE_NO_INSTANCE_AVAILABLE(status) \
+ ({ NTSTATUS _s = (status); \
+ _s == STATUS_INSTANCE_NOT_AVAILABLE \
+ || _s == STATUS_PIPE_NOT_AVAILABLE \
+ || _s == STATUS_PIPE_BUSY; })
+
fhandler_fifo::fhandler_fifo ():
fhandler_base (), read_ready (NULL), write_ready (NULL),
listen_client_thr (NULL), lct_termination_evt (NULL), nhandlers (0),
@@ -543,28 +549,32 @@ fhandler_fifo::open (int flags, mode_t)
listen_client thread is running. Then signal write_ready. */
if (writer)
{
- if (!wait (read_ready))
- {
- res = error_errno_set;
- goto out;
- }
- NTSTATUS status = open_pipe ();
- if (!NT_SUCCESS (status))
- {
- debug_printf ("create of writer failed");
- __seterrno_from_nt_status (status);
- res = error_errno_set;
- goto out;
- }
- else if (!arm (write_ready))
+ while (1)
{
- res = error_set_errno;
- goto out;
- }
- else
- {
- set_pipe_non_blocking (get_handle (), true);
- res = success;
+ if (!wait (read_ready))
+ {
+ res = error_errno_set;
+ goto out;
+ }
+ NTSTATUS status = open_pipe ();
+ if (NT_SUCCESS (status))
+ {
+ set_pipe_non_blocking (get_handle (), true);
+ if (!arm (write_ready))
+ res = error_set_errno;
+ else
+ res = success;
+ goto out;
+ }
+ else if (STATUS_PIPE_NO_INSTANCE_AVAILABLE (status))
+ Sleep (1);
+ else
+ {
+ debug_printf ("create of writer failed");
+ __seterrno_from_nt_status (status);
+ res = error_errno_set;
+ goto out;
+ }
}
}
out:
--
2.17.0
[PATCH 01/14] Cygwin: FIFO: rename client[] to fc_handler[]
The word "client" suggests something that holds a handle to the client
side of the pipe (in Windows terminology). But our
fifo_client_handlers hold a handle the server side of the pipe, and
they *connect* to clients.
---
winsup/cygwin/fhandler.h | 20 +++
winsup/cygwin/fhandler_fifo.cc | 96 +-
winsup/cygwin/select.cc| 2 +-
3 files changed, 59 insertions(+), 59 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index bc66377cd..2898db4df 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1269,8 +1269,8 @@ class fhandler_fifo: public fhandler_base
HANDLE lct_termination_evt;
UNICODE_STRING pipe_name;
WCHAR pipe_name_buf[CYGWIN_FIFO_PIPE_NAME_LEN + 1];
- fifo_client_handler client[MAX_CLIENTS];
- int nclients, nconnected;
+ fifo_client_handler fc_handler[MAX_CLIENTS];
+ int nhandlers, nconnected;
af_unix_spinlock_t _fifo_client_lock;
bool _duplexer;
bool __reg2 wait (HANDLE);
@@ -1278,15 +1278,15 @@ class fhandler_fifo: public fhandler_base
HANDLE create_pipe_instance (bool);
NTSTATUS open_pipe ();
int disconnect_and_reconnect (int);
- int add_client ();
+ int add_client_handler ();
bool listen_client ();
public:
fhandler_fifo ();
bool hit_eof ();
- int get_nclients () const { return nclients; }
+ int get_nhandlers () const { return nhandlers; }
HANDLE& get_handle () { return fhandler_base::get_handle (); }
- HANDLE get_handle (int i) const { return client[i].fh->get_handle (); }
- bool is_connected (int i) const { return client[i].state == fc_connected; }
+ HANDLE get_handle (int i) const { return fc_handler[i].fh->get_handle (); }
+ bool is_connected (int i) const { return fc_handler[i].state ==
fc_connected; }
PUNICODE_STRING get_pipe_name ();
DWORD listen_client_thread ();
void fifo_client_lock () { _fifo_client_lock.lock (); }
@@ -1305,8 +1305,8 @@ public:
void clear_readahead ()
{
fhandler_base::clear_readahead ();
-for (int i = 0; i < nclients; i++)
- client[i].fh->clear_readahead ();
+for (int i = 0; i < nhandlers; i++)
+ fc_handler[i].fh->clear_readahead ();
}
select_record *select_read (select_stuff *);
select_record *select_write (select_stuff *);
@@ -1326,8 +1326,8 @@ public:
void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_fifo));
fhandler_fifo *fhf = new (ptr) fhandler_fifo (ptr);
copyto (fhf);
-for (int i = 0; i < nclients; i++)
- fhf->client[i].fh = client[i].fh->clone ();
+for (int i = 0; i < nhandlers; i++)
+ fhf->fc_handler[i].fh = fc_handler[i].fh->clone ();
return fhf;
}
};
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 68b9e7717..901696444 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -32,7 +32,7 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
fhandler_fifo::fhandler_fifo ():
fhandler_base (), read_ready (NULL), write_ready (NULL),
- listen_client_thr (NULL), lct_termination_evt (NULL), nclients (0),
+ listen_client_thr (NULL), lct_termination_evt (NULL), nhandlers (0),
nconnected (0), _duplexer (false)
{
pipe_name_buf[0] = L'\0';
@@ -146,7 +146,7 @@ fhandler_fifo::disconnect_and_reconnect (int i)
{
NTSTATUS status;
IO_STATUS_BLOCK io;
- HANDLE ph = client[i].fh->get_handle ();
+ HANDLE ph = fc_handler[i].fh->get_handle ();
status = NtFsControlFile (ph, NULL, NULL, NULL, &io, FSCTL_PIPE_DISCONNECT,
NULL, 0, NULL, 0);
@@ -159,10 +159,10 @@ fhandler_fifo::disconnect_and_reconnect (int i)
__seterrno_from_nt_status (status);
return -1;
}
- set_pipe_non_blocking (client[i].fh->get_handle (), false);
- if (client[i].connect () < 0)
+ set_pipe_non_blocking (fc_handler[i].fh->get_handle (), false);
+ if (fc_handler[i].connect () < 0)
return -1;
- if (client[i].state == fc_connected)
+ if (fc_handler[i].state == fc_connected)
nconnected++;
return 0;
}
@@ -198,9 +198,9 @@ fhandler_fifo::npfs_handle (HANDLE &nph)
}
/* Called when a FIFO is first opened for reading and again each time
- a new client is needed. Each pipe instance is created in blocking
- mode so that we can easily wait for a connection. After it is
- connected, it is put in nonblocking mode. */
+ a new client handler is needed. Each pipe instance is created in
+ blocking mode so that we can easily wait for a connection. After
+ it is connected, it is put in nonblocking mode. */
HANDLE
fhandler_fifo::create_pipe_instance (bool first)
{
@@ -271,13 +271,13 @@ fhandler_fifo::open_pipe ()
}
int
-fhandler_fifo::add_client ()
+fhandler_fifo::add_client_handler ()
{
fifo_client_handler fc;
fhandler_base *fh;
- bool first = (nclients == 0);
+ bool first = (nhandlers == 0);
- if (nclients == MAX_CLIENTS)
+ if (nhandlers == MAX_CLIENTS)
{
set_errno (EMFI
[PATCH 08/14] Cygwin: FIFO: fix fifo_client_handler::close
Make sure that fhandler_base::close rather than fhandler_fifo::close
is called on the fhandler. Also, delete the fhandler, since we
allocated it.
---
winsup/cygwin/fhandler_fifo.cc | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 2da579b95..f9796f300 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -809,7 +809,10 @@ fifo_client_handler::close ()
int res = 0;
if (fh)
-res = fh->close ();
+{
+ res = fh->fhandler_base::close ();
+ delete fh;
+}
if (connect_evt)
CloseHandle (connect_evt);
if (dummy_evt)
--
2.17.0
[PATCH 05/14] Cygwin: FIFO: fix the error checking in raw_read
If the pipe is empty, we can get either ERROR_NO_DATA or
ERROR_PIPE_LISTENING.
---
winsup/cygwin/fhandler_fifo.cc | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 764420ffd..2da579b95 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -749,19 +749,16 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
fifo_client_unlock ();
return;
}
- /* In the duplex case with no data, we seem to get nread
- == -1 with ERROR_PIPE_LISTENING on the first attempt to
- read from the duplex pipe (fc_handler[0]), and nread == 0
- on subsequent attempts. */
+ /* If the pipe is empty, we usually get nread == -1 with
+ ERROR_NO_DATA or ERROR_PIPE_LISTENING. An exception is
+ that in the duplex case we may get nread == 0 when we
+ attempt to read from the duplex pipe (fc_handler[0]). */
else if (nread < 0)
switch (GetLastError ())
{
case ERROR_NO_DATA:
- break;
case ERROR_PIPE_LISTENING:
- if (_duplexer && i == 0)
- break;
- /* Fall through. */
+ break;
default:
fifo_client_unlock ();
goto errout;
--
2.17.0
[PATCH 09/14] Cygwin: FIFO: fix the use of the read_ready event
Make read_ready a manual reset event. It should always be set shortly
after startup of the listen_client thread and remain set until the
thread terminates. (We don't want writers to connect without being
recorded in the client handler list.)
Remove the unnecessary code that checks for read_ready when a reader
is opening.
---
winsup/cygwin/fhandler_fifo.cc | 51 +++---
1 file changed, 16 insertions(+), 35 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index f9796f300..479021e8e 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -344,6 +344,8 @@ fhandler_fifo::listen_client ()
DWORD
fhandler_fifo::listen_client_thread ()
{
+ DWORD ret = -1;
+
while (1)
{
bool found;
@@ -360,7 +362,7 @@ fhandler_fifo::listen_client_thread ()
if (disconnect_and_reconnect (i) < 0)
{
fifo_client_unlock ();
- goto errout;
+ goto out;
}
else
/* Recheck fc_handler[i].state. */
@@ -383,23 +385,27 @@ fhandler_fifo::listen_client_thread ()
res = add_client_handler ();
fifo_client_unlock ();
if (res < 0)
- goto errout;
+ goto out;
else if (!found)
continue;
+ /* Allow a writer to open. */
if (!arm (read_ready))
{
__seterrno ();
- goto errout;
+ goto out;
}
/* Wait for a client to connect. */
wait_ret = WaitForMultipleObjects (nhandlers + 1, w, false, INFINITE);
i = wait_ret - WAIT_OBJECT_0;
if (i < 0 || i > nhandlers)
- goto errout;
- else if (i == nhandlers) /* Reader is closing. */
- return 0;
+ goto out;
+ else if (i == nhandlers) /* Thread termination requested. */
+ {
+ ret = 0;
+ goto out;
+ }
else
{
fifo_client_lock ();
@@ -410,9 +416,9 @@ fhandler_fifo::listen_client_thread ()
yield ();
}
}
-errout:
+out:
ResetEvent (read_ready);
- return -1;
+ return ret;
}
int
@@ -457,7 +463,7 @@ fhandler_fifo::open (int flags, mode_t)
char npbuf[MAX_PATH];
__small_sprintf (npbuf, "r-event.%08x.%016X", get_dev (), get_ino ());
- if (!(read_ready = CreateEvent (sa_buf, duplexer, false, npbuf)))
+ if (!(read_ready = CreateEvent (sa_buf, true, false, npbuf)))
{
debug_printf ("CreateEvent for %s failed, %E", npbuf);
res = error_set_errno;
@@ -523,32 +529,7 @@ fhandler_fifo::open (int flags, mode_t)
res = error_errno_set;
goto out;
}
- /* Wait for the listen_client thread to signal read_ready. This
-should be quick. */
- HANDLE w[2] = { listen_client_thr, read_ready };
- switch (WaitForMultipleObjects (2, w, FALSE, INFINITE))
- {
- case WAIT_OBJECT_0:
- debug_printf ("listen_client_thread exited unexpectedly");
- DWORD err;
- GetExitCodeThread (listen_client_thr, &err);
- __seterrno_from_win_error (err);
- res = error_errno_set;
- goto out;
- break;
- case WAIT_OBJECT_0 + 1:
- if (!arm (read_ready))
- {
- res = error_set_errno;
- goto out;
- }
- break;
- default:
- res = error_set_errno;
- goto out;
- break;
- }
- if (!duplexer && !wait (write_ready))
+ else if (!duplexer && !wait (write_ready))
{
res = error_errno_set;
goto out;
--
2.17.0
[PATCH 12/14] Cygwin: FIFO: start the listen_client thread when duping a reader
Otherwise it doesn't get started until the dup'd fd tries to read,
which delays client connections.
---
winsup/cygwin/fhandler_fifo.cc | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index a3ecbeb4a..fe4c67bdf 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -853,19 +853,20 @@ fhandler_fifo::close ()
int
fhandler_fifo::dup (fhandler_base *child, int flags)
{
+ int ret = -1;
+ fhandler_fifo *fhf = NULL;
+
if (fhandler_base::dup (child, flags))
-{
- __seterrno ();
- return -1;
-}
- fhandler_fifo *fhf = (fhandler_fifo *) child;
+goto out;
+
+ fhf = (fhandler_fifo *) child;
if (!DuplicateHandle (GetCurrentProcess (), read_ready,
GetCurrentProcess (), &fhf->read_ready,
0, true, DUPLICATE_SAME_ACCESS))
{
fhf->close ();
__seterrno ();
- return -1;
+ goto out;
}
if (!DuplicateHandle (GetCurrentProcess (), write_ready,
GetCurrentProcess (), &fhf->write_ready,
@@ -874,7 +875,7 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
CloseHandle (fhf->read_ready);
fhf->close ();
__seterrno ();
- return -1;
+ goto out;
}
for (int i = 0; i < nhandlers; i++)
{
@@ -895,13 +896,16 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
CloseHandle (fhf->write_ready);
fhf->close ();
__seterrno ();
- return -1;
+ goto out;
}
}
fhf->listen_client_thr = NULL;
fhf->lct_termination_evt = NULL;
fhf->fifo_client_unlock ();
- return 0;
+ if (!reader || fhf->listen_client ())
+ret = 0;
+out:
+ return ret;
}
void
--
2.17.0
[PATCH 14/14] Cygwin: FIFO: fix and simplify listen_client_thread
Remove fifo_client_handler::connect and move its code into
listen_client_thread. That way we can check the return status when a
client handler's connect_evt is signaled. Previously we incorrectly
assumed there was a successful connection.
Also simplify listen_client_thread in the following ways:
- Replace fhandler_fifo::disconnect_and_reconnect by a new
delete_client_handler method. Now we just delete invalid client
handlers rather than trying to re-use them.
- Try to maintain a client handler list that consists of connected
client handlers and exactly one that is listening for a connection.
This allows us to call WaitForMultipleObjects with only two wait
objects.
- Remove 'dummy_evt' from the fifo_client_handler struct; it is no
longer needed.
- On exit from listen_client_thread, delete the "extra" (listening)
client handler. Otherwise there could be a connection that doesn't
get recorded in the client handler list. This could happen when a
file descriptor is being duplicated.
---
winsup/cygwin/fhandler.h | 11 +-
winsup/cygwin/fhandler_fifo.cc | 251 ++---
2 files changed, 109 insertions(+), 153 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index fd205a6db..8fb176b24 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1250,13 +1250,10 @@ struct fifo_client_handler
fhandler_base *fh;
fifo_client_connect_state state;
HANDLE connect_evt;
- HANDLE dummy_evt;/* Never signaled. */
- fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL),
- dummy_evt (NULL) {}
+ fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL) {}
fifo_client_handler (fhandler_base *_fh, fifo_client_connect_state _state,
- HANDLE _connect_evt, HANDLE _dummy_evt)
-: fh (_fh), state (_state), connect_evt (_connect_evt),
- dummy_evt (_dummy_evt) {}
+ HANDLE _connect_evt)
+: fh (_fh), state (_state), connect_evt (_connect_evt) {}
int connect ();
int close ();
};
@@ -1278,8 +1275,8 @@ class fhandler_fifo: public fhandler_base
NTSTATUS npfs_handle (HANDLE &);
HANDLE create_pipe_instance (bool);
NTSTATUS open_pipe ();
- int disconnect_and_reconnect (int);
int add_client_handler ();
+ void delete_client_handler (int);
bool listen_client ();
int stop_listen_client ();
public:
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index ce078d74d..abbb29a61 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -121,62 +121,6 @@ set_pipe_non_blocking (HANDLE ph, bool nonblocking)
debug_printf ("NtSetInformationFile(FilePipeInformation): %y", status);
}
-/* The pipe instance is always in blocking mode when this is called. */
-int
-fifo_client_handler::connect ()
-{
- NTSTATUS status;
- IO_STATUS_BLOCK io;
-
- if (connect_evt)
-ResetEvent (connect_evt);
- else if (!(connect_evt = create_event ()))
-return -1;
- status = NtFsControlFile (fh->get_handle (), connect_evt, NULL, NULL, &io,
- FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
- switch (status)
-{
-case STATUS_PENDING:
-case STATUS_PIPE_LISTENING:
- state = fc_connecting;
- break;
-case STATUS_PIPE_CONNECTED:
- state = fc_connected;
- set_pipe_non_blocking (fh->get_handle (), true);
- break;
-default:
- __seterrno_from_nt_status (status);
- return -1;
-}
- return 0;
-}
-
-int
-fhandler_fifo::disconnect_and_reconnect (int i)
-{
- NTSTATUS status;
- IO_STATUS_BLOCK io;
- HANDLE ph = fc_handler[i].fh->get_handle ();
-
- status = NtFsControlFile (ph, NULL, NULL, NULL, &io, FSCTL_PIPE_DISCONNECT,
- NULL, 0, NULL, 0);
- /* Short-lived. Don't use cygwait. We don't want to be interrupted. */
- if (status == STATUS_PENDING
- && NtWaitForSingleObject (ph, FALSE, NULL) == WAIT_OBJECT_0)
-status = io.Status;
- if (!NT_SUCCESS (status))
-{
- __seterrno_from_nt_status (status);
- return -1;
-}
- set_pipe_non_blocking (fc_handler[i].fh->get_handle (), false);
- if (fc_handler[i].connect () < 0)
-return -1;
- if (fc_handler[i].state == fc_connected)
-nconnected++;
- return 0;
-}
-
NTSTATUS
fhandler_fifo::npfs_handle (HANDLE &nph)
{
@@ -283,41 +227,49 @@ fhandler_fifo::open_pipe ()
int
fhandler_fifo::add_client_handler ()
{
+ int ret = -1;
fifo_client_handler fc;
fhandler_base *fh;
+ HANDLE ph = NULL;
bool first = (nhandlers == 0);
if (nhandlers == MAX_CLIENTS)
{
set_errno (EMFILE);
- return -1;
+ goto out;
}
- if (!(fc.dummy_evt = create_event ()))
-return -1;
+ if (!(fc.connect_evt = create_event ()))
+goto out;
if (!(fh = build_fh_dev (dev (
{
set_errno (EMFILE);
- return -1;
+ goto out;
}
- fc.fh = fh;
- HAN
[PATCH v2 13/14] Cygwin: FIFO: improve raw_write
Don't set the write end of the pipe to non-blocking mode if the FIFO
is opened in blocking mode.
In fhandler_fifo::raw_write in blocking mode, wait for the write to
complete rather than returning -1 with EAGAIN.
If the amount to write is large, write in smaller chunks (of size
determined by a new data member max_atomic_write), as in
fhandler_base_overlapped.
For convenience, add two new NTSTATUS codes, STATUS_THREAD_SIGNALED
and STATUS_THREAD_CANCELED, to ntdll.h.
---
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_fifo.cc | 95 +++---
winsup/cygwin/ntdll.h | 4 +-
3 files changed, 80 insertions(+), 20 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index aea02c2b3..fd205a6db 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1273,6 +1273,7 @@ class fhandler_fifo: public fhandler_base
int nhandlers, nconnected;
af_unix_spinlock_t _fifo_client_lock;
bool reader, writer, duplexer;
+ size_t max_atomic_write;
bool __reg2 wait (HANDLE);
NTSTATUS npfs_handle (HANDLE &);
HANDLE create_pipe_instance (bool);
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index fe4c67bdf..d2f8b99bc 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -39,7 +39,8 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
fhandler_fifo::fhandler_fifo ():
fhandler_base (), read_ready (NULL), write_ready (NULL),
listen_client_thr (NULL), lct_termination_evt (NULL), nhandlers (0),
- nconnected (0), reader (false), writer (false), duplexer (false)
+ nconnected (0), reader (false), writer (false), duplexer (false),
+ max_atomic_write (DEFAULT_PIPEBUFSIZE)
{
pipe_name_buf[0] = L'\0';
need_fork_fixup (true);
@@ -559,7 +560,7 @@ fhandler_fifo::open (int flags, mode_t)
NTSTATUS status = open_pipe ();
if (NT_SUCCESS (status))
{
- set_pipe_non_blocking (get_handle (), true);
+ set_pipe_non_blocking (get_handle (), flags & O_NONBLOCK);
if (!arm (write_ready))
res = error_set_errno;
else
@@ -661,28 +662,84 @@ ssize_t __reg3
fhandler_fifo::raw_write (const void *ptr, size_t len)
{
ssize_t ret = -1;
- NTSTATUS status;
+ size_t nbytes = 0, chunk;
+ NTSTATUS status = STATUS_SUCCESS;
IO_STATUS_BLOCK io;
+ HANDLE evt = NULL;
- status = NtWriteFile (get_handle (), NULL, NULL, NULL, &io,
- (PVOID) ptr, len, NULL, NULL);
- if (NT_SUCCESS (status))
+ if (len <= max_atomic_write)
+chunk = len;
+ else if (is_nonblocking ())
+chunk = len = max_atomic_write;
+ else
+chunk = max_atomic_write;
+
+ /* Create a wait event if the FIFO is in blocking mode. */
+ if (!is_nonblocking () && !(evt = CreateEvent (NULL, false, false, NULL)))
+return -1;
+
+ /* Write in chunks, accumulating a total. If there's an error, just
+ return the accumulated total unless the first write fails, in
+ which case return -1. */
+ while (nbytes < len)
{
- /* NtWriteFile returns success with # of bytes written == 0 in
-case writing on a non-blocking pipe fails if the pipe buffer
-is full. */
- if (io.Information == 0)
- set_errno (EAGAIN);
+ ULONG_PTR nbytes_now = 0;
+ size_t left = len - nbytes;
+ size_t len1;
+ if (left > chunk)
+ len1 = chunk;
else
- ret = io.Information;
-}
- else if (STATUS_PIPE_IS_CLOSED (status))
-{
- set_errno (EPIPE);
- raise (SIGPIPE);
+ len1 = left;
+ nbytes_now = 0;
+ status = NtWriteFile (get_handle (), evt, NULL, NULL, &io,
+ (PVOID) ptr, len1, NULL, NULL);
+ if (evt && status == STATUS_PENDING)
+ {
+ DWORD waitret = cygwait (evt, cw_infinite, cw_cancel | cw_sig_eintr);
+ switch (waitret)
+ {
+ case WAIT_OBJECT_0:
+ status = io.Status;
+ break;
+ case WAIT_SIGNALED:
+ status = STATUS_THREAD_SIGNALED;
+ break;
+ case WAIT_CANCELED:
+ status = STATUS_THREAD_CANCELED;
+ break;
+ default:
+ break;
+ }
+ }
+ if (NT_SUCCESS (status))
+ {
+ nbytes_now = io.Information;
+ /* NtWriteFile returns success with # of bytes written == 0
+if writing on a non-blocking pipe fails because the pipe
+buffer doesn't have sufficient space. */
+ if (nbytes_now == 0)
+ set_errno (EAGAIN);
+ ptr = ((char *) ptr) + chunk;
+ nbytes += nbytes_now;
+ }
+ else if (STATUS_PIPE_IS_CLOSED (status))
+ {
+ set_errno (EPIPE);
+ raise (SIGPIPE);
+ }
+ else
+ __seterrno_from_nt_status (status);
+ if (nbytes_now == 0)
+ len = 0;/* Terminate loop. */
+
Re: [PATCH 00/14] FIFO bug fixes and code simplifications
On 4/17/2019 1:23 PM, Achim Gratz wrote: > Corinna Vinschen writes: >> Pushed with v2 of patch 13. Developer snaps should be up shortly. > > I gave the snapshot some testing today. > > The good news is that the named FIFO branch of our code works correctly > again and is faster than going through pseudo-file STDIO pipes. > > The bad news is that there is still some problem that seems to hit while > forking. I've got an empty stackdump file from sh (which gets used when > exec'ing due to the way perl implements that) a few times and also one > error message about a terminated thread due to "Windows WFSO error 6" > (hilariously the output file was produced correctly in that case). But > most of the time the processes in my data pipeline would all have execed > correctly, but then none of them ever gets runnable again. So this > seems to be something of a race around the exec. If I kill the stalled > processes and start the same commands again, then everything works as it > should most of the time. Thanks for testing. I have STCs for fork and exec that I used when first writing the code, and I forgot to retest those after the recent changes. I just tried, and the fork test succeeds but the exec test fails. I'll try to debug that. Ken
[PATCH] Cygwin: FIFO: avoid hang after exec
Define fhandler:fifo::fixup_after_exec, which sets listen_client_thr
and lct_termination_evt to NULL. This forces the listen_client thread
to restart on the first attempt to read after an exec. Previously the
exec'd process could hang in fhandler_fifo::raw_read.
---
winsup/cygwin/fhandler.h | 1 +
winsup/cygwin/fhandler_fifo.cc | 9 +
2 files changed, 10 insertions(+)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 8fb176b24..da007ee45 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1301,6 +1301,7 @@ public:
ssize_t __reg3 raw_write (const void *ptr, size_t ulen);
bool arm (HANDLE h);
void fixup_after_fork (HANDLE);
+ void fixup_after_exec ();
int __reg2 fstatvfs (struct statvfs *buf);
void clear_readahead ()
{
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 1d02adbaa..bc9c23998 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -942,6 +942,15 @@ fhandler_fifo::fixup_after_fork (HANDLE parent)
fifo_client_unlock ();
}
+void
+fhandler_fifo::fixup_after_exec ()
+{
+ fhandler_base::fixup_after_exec ();
+ listen_client_thr = NULL;
+ lct_termination_evt = NULL;
+ fifo_client_unlock ();
+}
+
void
fhandler_fifo::set_close_on_exec (bool val)
{
--
2.17.0
Re: [PATCH 00/14] FIFO bug fixes and code simplifications
On 4/17/2019 2:58 PM, Achim Gratz wrote: > Ken Brown writes: >> Thanks for testing. I have STCs for fork and exec that I used when first >> writing the code, and I forgot to retest those after the recent changes. I >> just >> tried, and the fork test succeeds but the exec test fails. I'll try to >> debug that. > > Take your time and thanks for working in that area. I've just sent an attempted fix. There may still be more changes needed; I just made a minimal change to make my STC succeed. Ken
Re: Cygwin patches (was Re: [PATCH 00/14] FIFO bug fixes and code simplifications)
On 4/18/2019 3:04 PM, Corinna Vinschen wrote: > Ken, you have an account on sware and check-in rights to the cygwin repo > anyway, so you don't really need me to push patches. > > If you feel up to the task, you can also go ahead and review and > ultimately push patches from others. This also applies to Takashi's > ConPTY patch as soon as he wants to go mainline, I can do that. > as well as Mark's > sched_[gs]etaffinity patches, after review. I'm not sure I'm qualified to review these, but maybe he won't wait until May :) Ken
[PATCH 3/5] Cygwin: FIFO: avoid WFMO error in listen_client_thread
Don't set lct_termination_evt to NULL too early in
fhandler_fifo::stop_listen_client. Doing so leads to an "Invalid
Handle" error in WFMO.
---
winsup/cygwin/fhandler_fifo.cc | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 0a6dc0591..0e4bf3aee 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -844,22 +844,24 @@ int
fhandler_fifo::stop_listen_client ()
{
int ret = 0;
- HANDLE evt = InterlockedExchangePointer (&lct_termination_evt, NULL);
- HANDLE thr = InterlockedExchangePointer (&listen_client_thr, NULL);
+ HANDLE thr, evt;
+
+ thr = InterlockedExchangePointer (&listen_client_thr, NULL);
if (thr)
{
- if (evt)
- SetEvent (evt);
+ if (lct_termination_evt)
+ SetEvent (lct_termination_evt);
WaitForSingleObject (thr, INFINITE);
DWORD err;
GetExitCodeThread (thr, &err);
if (err)
{
ret = -1;
- debug_printf ("listen_client_thread exited with error, %E");
+ debug_printf ("listen_client_thread exited with error");
}
CloseHandle (thr);
}
+ evt = InterlockedExchangePointer (&lct_termination_evt, NULL);
if (evt)
CloseHandle (evt);
return ret;
--
2.17.0
[PATCH 4/5] Cygwin: FIFO: close connect_evt handles as soon as possible
Keeping them open too long can cause an attempt to close them twice
after a fork or exec.
---
winsup/cygwin/fhandler_fifo.cc | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 0e4bf3aee..3ee307bcc 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -363,6 +363,7 @@ fhandler_fifo::listen_client_thread ()
break;
}
}
+ HANDLE evt = NULL;
switch (status)
{
case STATUS_SUCCESS:
@@ -371,6 +372,9 @@ fhandler_fifo::listen_client_thread ()
fc.state = fc_connected;
nconnected++;
set_pipe_non_blocking (fc.fh->get_handle (), true);
+ evt = InterlockedExchangePointer (&fc.connect_evt, NULL);
+ if (evt)
+ CloseHandle (evt);
fifo_client_unlock ();
break;
case STATUS_PIPE_LISTENING:
@@ -400,6 +404,8 @@ fhandler_fifo::listen_client_thread ()
}
}
out:
+ if (ret < 0)
+debug_printf ("exiting lct with error, %E");
ResetEvent (read_ready);
return ret;
}
@@ -829,14 +835,15 @@ int
fifo_client_handler::close ()
{
int res = 0;
+ HANDLE evt = InterlockedExchangePointer (&connect_evt, NULL);
+ if (evt)
+CloseHandle (evt);
if (fh)
{
res = fh->fhandler_base::close ();
delete fh;
}
- if (connect_evt)
-CloseHandle (connect_evt);
return res;
}
@@ -913,11 +920,7 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
if (!DuplicateHandle (GetCurrentProcess (), fc_handler[i].fh->get_handle
(),
GetCurrentProcess (),
&fhf->fc_handler[i].fh->get_handle (),
- 0, true, DUPLICATE_SAME_ACCESS)
- || !DuplicateHandle (GetCurrentProcess (), fc_handler[i].connect_evt,
- GetCurrentProcess (),
- &fhf->fc_handler[i].connect_evt,
- 0, true, DUPLICATE_SAME_ACCESS))
+ 0, true, DUPLICATE_SAME_ACCESS))
{
CloseHandle (fhf->read_ready);
CloseHandle (fhf->write_ready);
--
2.17.0
