test
please ignore
Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().
On Mar 23 21:42, Takashi Yano via Cygwin-patches wrote: > On Tue, 23 Mar 2021 21:32:12 +0900 > Takashi Yano wrote: > > I try to check run.exe behaviour and noticed that > > run cmd.exe > > and > > run cat.exe > > does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these > > work in 3.1.7. > > In obove cases, cmd.exe and cat.exe is running in *hidden* console, > therefore nothing is shown. Right? Yes, that's it. In my other reply I wrote something about running without console, but in fact that's running without *visible* console, so, yeah, setting up the hidden console is it's task. Corinna
Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().
On Mar 23 21:32, Takashi Yano via Cygwin-patches wrote: > On Tue, 23 Mar 2021 13:17:16 +0100 > Corinna Vinschen wrote: > > On Mar 23 20:57, Takashi Yano via Cygwin-patches wrote: > > > Corinna Vinschen wrote: > > > > > > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote: > > > > > > > > And also, following cygwin apps/dlls call GetStdHandle(): > > > > > > > > ccmake.exe > > > > > > > > cmake.exe > > > > > > > > cpack.exe > > > > > > > > ctest.exe > > > > > > > > run.exe > > > > > > > > run creates its own conin/conout handles to create a hidden console. > > > > The code calling GetStdHandle() is only for debug purposes and never > > > > built into the executable. > > > > Sorry, but this was utterly wrong. run calls GetStdHandle, then > > overwrites the handles, but only if it doesn't already is attached to a > > console. > > > > > > Looks right to me. If we patch cmake to do the right thing, do we still > > > > need this patch, Takashi? > > > > > > I don't think so. If all is well with current code, nothing to be fixed. > > > > How do you evaluate this in light of the run behaviour above? > > I try to check run.exe behaviour and noticed that > run cmd.exe > and > run cat.exe > does not work with cygwin 3.0.7 and 3.2.0 (TEST) while these > work in 3.1.7. > > Is this expected behaviour? The problem is that I never used run. I can't actually tell what exactly is expected. I *think* run was intended to start Cygwin applications without console window in the first place, not native Windows apps, but I could be wrong. I don't even know if anybody is actually, seriously using it. The biggest problem with run is probably that it's unmaintained for a long time, and seeing code comments like /* Note: we do it this way instead of using GetConsoleWindow() * because we want it to work on < w2k. */ doesn't actually make me confident in its viability for any purpose... Corinna
Re: [PATCH] Cygwin: pty: Rename input/output named pipes.
On Mar 23 20:50, Takashi Yano via Cygwin-patches wrote: > - Currently, names of output pipes are "pty%d-to-master" and "pty%d- > to-master-cyg" and names of input pipes are "pty%d-to-slave" and > "pty%d-from-master". With this patch, these pipes are renamed to > "pty%d-to-master-nat", "pty%d-to-master", "pty%d-from-master-nat" > and "pty%d-from-master" respectively. > --- > winsup/cygwin/fhandler_tty.cc | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().
On Mar 23 20:57, Takashi Yano via Cygwin-patches wrote: > Corinna Vinschen wrote: > > > > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote: > > > > > > And also, following cygwin apps/dlls call GetStdHandle(): > > > > > > ccmake.exe > > > > > > cmake.exe > > > > > > cpack.exe > > > > > > ctest.exe > > > > > > run.exe > > > > run creates its own conin/conout handles to create a hidden console. > > The code calling GetStdHandle() is only for debug purposes and never > > built into the executable. Sorry, but this was utterly wrong. run calls GetStdHandle, then overwrites the handles, but only if it doesn't already is attached to a console. > > Looks right to me. If we patch cmake to do the right thing, do we still > > need this patch, Takashi? > > I don't think so. If all is well with current code, nothing to be fixed. How do you evaluate this in light of the run behaviour above? Thanks, Corinna
Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().
[CC Marco, CC Jan] On Mar 22 13:02, Ken Brown via Cygwin-patches wrote: > [Still CC Marco] > > On 3/22/2021 7:43 AM, Corinna Vinschen via Cygwin-patches wrote: > > [CC Marco] > > > > On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote: > > > On Sun, 21 Mar 2021 17:44:27 +0900 > > > Takashi Yano wrote: > > > > [...] > > > > However, following cygwin apps/dlls call _get_osfhandle(): > > > > ccmake.exe > > > > cmake.exe > > > > cpack.exe > > > > ctest.exe > > > > ddrescue.exe I'm pretty sure ddrescue needs the osfhandle just to access raw block devices. > > > > And also, following cygwin apps/dlls call GetStdHandle(): > > > > ccmake.exe > > > > cmake.exe > > > > cpack.exe > > > > ctest.exe > > > > run.exe run creates its own conin/conout handles to create a hidden console. The code calling GetStdHandle() is only for debug purposes and never built into the executable. > > > > cygusb0.dll This lib tries to access USB devices only. > > > > tk86.dll Not sure about this one. In theory this shouldn't happen, given our tk is built against X11, not against the Windows GUI. Jan, can you please check where and why tk86.dll calls GetStdHandle. I found a few places in the source where GetStdHandle is called, but it's not clear to me which one is called. > Out of curiosity, I took a quick glance at the cmake code. It appears that > this code is designed to support running cmake in a Console. I don't think > that should be needed any more, if it ever was. > [...] > I think the following might suffice (untested): > > --- a/Source/kwsys/Terminal.c > +++ b/Source/kwsys/Terminal.c > @@ -10,7 +10,7 @@ > #endif > > /* Configure support for this platform. */ > -#if defined(_WIN32) || defined(__CYGWIN__) > +#if defined(_WIN32) > # define KWSYS_TERMINAL_SUPPORT_CONSOLE > #endif > #if !defined(_WIN32) Looks right to me. If we patch cmake to do the right thing, do we still need this patch, Takashi? Thanks, Corinna
Re: [PATCH] Cygwin: pty: Rename input named pipes.
On Mar 23 09:38, Takashi Yano via Cygwin-patches wrote: > On Mon, 22 Mar 2021 12:49:20 +0100 > Corinna Vinschen wrote: > > Hi Takashi, > > > > On Mar 21 12:59, Takashi Yano via Cygwin-patches wrote: > > > - Currently, the name of input pipe is "pty-from-master" for > > > cygwin process, and "pty-to-slave" for non-cygwin process. > > > These are not only inconsistent with output pipes but also very > > > confusing. > > > With this patch, these are renamed to "pty-from-master-cyg" > > > and "pty-from-master" respectively. > > > --- > > > winsup/cygwin/fhandler_tty.cc | 2 +- > > > winsup/cygwin/tty.cc | 4 ++-- > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > Actually... wouldn't it make more sense to call the Cygwin pipe > > > > pty%d-from-master / pty%d-to-slave > > > > and the non-Cygwin one something like > > > > pty%d-from-master-nat / pty%d-to-slave-nat > > > > ? > > > > After all, Cygwin is the norm, and non-Cygwin is the exception. > > > > On second thought, this would also make sense for thr fhandler methods, > > i. e. > > > > get_output_handle / get_output_handle_cyg > > > > vs. > > > > get_output_handle_nat / get_output_handle > > > > Probably the fhandler stuff is too much renaming for this release, > > but we should do this for the next one, I think. > > I basically agree. However, renaming them consistently is > too much for 3.2.0 release as you mentioned. So, IMHO, it > is better to apply this patch once for 3.2.0 release and > then fully rename them for the next one. > > What do you think? I thought of renaming the pipes in this release, since you're already renaimg it anyway. Renaming the fhandler members and methods could take place in the next release. Do you prefer to rename pipes and fhandler methods in a single release? Thanks, Corinna
Re: [PATCH 1/2] Treat Windows Store's "app execution aliases" as symbolic links
On Mar 22 22:54, Hans-Bernhard Bröker wrote: > Am 22.03.2021 um 16:22 schrieb Johannes Schindelin: > > One of those under-documented reparse point types is the WSL symbolic > > link, which you will notice are supported in Cygwin, removing quite some > > sway from your argument... > > I notice no such thing right now, running the currently available release > version 3.1.7: > > stat: cannot stat '//wsl$/Debian/home/hbbro/link_to_a': Input/output error What type of WSL symlink is that? Cygwin reads WSL 1 symlinks(*) and creates them by default in symlink(2) cals since Cygwin 3.1.5. (*) IO_REPARSE_TAG_LX_SYMLINK 0xa01d Corinna
Re: [PATCH] Cygwin: pty: Rename input named pipes.
Hi Takashi, On Mar 21 12:59, Takashi Yano via Cygwin-patches wrote: > - Currently, the name of input pipe is "pty-from-master" for > cygwin process, and "pty-to-slave" for non-cygwin process. > These are not only inconsistent with output pipes but also very > confusing. > With this patch, these are renamed to "pty-from-master-cyg" > and "pty-from-master" respectively. > --- > winsup/cygwin/fhandler_tty.cc | 2 +- > winsup/cygwin/tty.cc | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Actually... wouldn't it make more sense to call the Cygwin pipe pty%d-from-master / pty%d-to-slave and the non-Cygwin one something like pty%d-from-master-nat / pty%d-to-slave-nat ? After all, Cygwin is the norm, and non-Cygwin is the exception. On second thought, this would also make sense for thr fhandler methods, i. e. get_output_handle / get_output_handle_cyg vs. get_output_handle_nat / get_output_handle Probably the fhandler stuff is too much renaming for this release, but we should do this for the next one, I think. Thanks, Corinna
Re: [PATCH 0/2] Return appropriate handle by _get_osfhandle() and GetStdHandle().
[CC Marco]
On Mar 22 08:07, Takashi Yano via Cygwin-patches wrote:
> On Sun, 21 Mar 2021 17:44:27 +0900
> Takashi Yano wrote:
> > On Sun, 21 Mar 2021 13:01:24 +0900
> > Takashi Yano wrote:
> > > Takashi Yano (2):
> > > Cygwin: syscalls.cc: Make _get_osfhandle() return appropriate handle.
> > > Cygwin: pty: Add hook for GetStdHandle() to return appropriate handle.
> > >
> > > winsup/cygwin/fhandler_tty.cc | 19 +++
> > > winsup/cygwin/syscalls.cc | 13 -
> > > 2 files changed, 31 insertions(+), 1 deletion(-)
> >
> > I submitted these patches, however, I still wonder if we really
> > need these patches. I cannot imagine the situation where handle
> > itself is needed rather than file descriptor.
> >
> > However, following cygwin apps/dlls call _get_osfhandle():
> > ccmake.exe
> > cmake.exe
> > cpack.exe
> > ctest.exe
> > ddrescue.exe
> >
> > And also, following cygwin apps/dlls call GetStdHandle():
> > ccmake.exe
> > cmake.exe
> > cpack.exe
> > ctest.exe
> > run.exe
> > cygusb0.dll
> > tk86.dll
> >
> > in my installation.
> >
> > Therefore, some of these apps/dlls may need these patches...
>
> I looked into cmake source and found the patch exactly for
> this issue. Therefore, it seems better to fix this.
>
> /* Get the Windows handle for a FILE stream. */
> static HANDLE kwsysTerminalGetStreamHandle(FILE* stream)
> {
> /* Get the C-library file descriptor from the stream. */
> int fd = fileno(stream);
>
> # if defined(__CYGWIN__)
> /* Cygwin seems to have an extra pipe level. If the file descriptor
> corresponds to stdout or stderr then obtain the matching windows
> handle directly. */
> if (fd == fileno(stdout)) {
> return GetStdHandle(STD_OUTPUT_HANDLE);
> } else if (fd == fileno(stderr)) {
> return GetStdHandle(STD_ERROR_HANDLE);
> }
> # endif
>
> /* Get the underlying Windows handle for the descriptor. */
> return (HANDLE)_get_osfhandle(fd);
> }
Why on earth is cmake using Windows functions on Cygwin at all???
It's not as if it actually requires Windows functionality on our
platform.
Marco, any input? Any chance to drop this Windows stuff from the Cygwin
code path in cmake?
Thanks,
Corinna
Re: [PATCH 1/2] Treat Windows Store's "app execution aliases" as symbolic links
Hi Johannes,
I'm not opposed to treat these applinks as symlinks. I have a
suggestion and a style nit, though.
On Mar 12 16:11, Johannes Schindelin via Cygwin-patches wrote:
> When the Windows Store version of Python is installed, so-called "app
> execution aliases" are put into the `PATH`. These are reparse points
> under the hood, with an undocumented format.
>
> We do know a bit about this format, though, as per the excellent analysis:
> https://www.tiraniddo.dev/2019/09/overview-of-windows-execution-aliases.html
>
> The first 4 bytes is the reparse tag, in this case it's
> 0x801B which is documented in the Windows SDK as
> IO_REPARSE_TAG_APPEXECLINK. Unfortunately there doesn't seem to
> be a corresponding structure, but with a bit of reverse
> engineering we can work out the format is as follows:
>
> Version: <4 byte integer>
> Package ID:
> Entry Point:
> Executable:
> Application Type:
Given we know this layout, what about introducing a matching struct,
like I did for REPARSE_LX_SYMLINK_BUFFER, for instructional purposes?
I. e.
typedef struct _REPARSE_APPEXECLINK_BUFFER
{
DWORD ReparseTag;
WORD ReparseDataLength;
WORD Reserved;
struct {
DWORD Version; /* Take member name with a grain of salt. */
WCHAR Strings[1];/* Four serialized, NUL-terminated WCHAR strings:
- Package ID
- Entry Point
- Executable Path
- Application Type
We're only interested in the Executable Path */
} AppExecLinkReparseBuffer;
} REPARSE_APPEXECLINK_BUFFER,*PREPARSE_APPEXECLINK_BUFFER;
> + else if (!remote && rp->ReparseTag == IO_REPARSE_TAG_APPEXECLINK)
> +{
> + /* App execution aliases are commonly used by Windows Store apps. */
> + WCHAR *buf = (WCHAR *)(rp->GenericReparseBuffer.DataBuffer + 4);
Analogue:
PREPARSE_APPEXECLINK_BUFFER rpl = (PREPARSE_APPEXECLINK_BUFFER) rp;
WCHAR *buf = rpl->AppExecLinkReparseBuffer.Strings;
Maybe use 'str' or 'strp' here, instead of buf?
> + for (int i = 0; i < 3 && size > 0; i++)
> +{
> + n = wcsnlen (buf, size - 1);
> + if (i == 2 && n > 0 && n < size)
> + {
> + RtlInitCountedUnicodeString (psymbuf, buf, n * sizeof(WCHAR));
^^^
space
Thanks,
Corinna
Re: [PATCH 1/2] Treat Windows Store's "app execution aliases" as symbolic links
On Mar 13 19:41, Joe Lowe via Cygwin-patches wrote: > Hi Johannes, > > I agree on the usefulness to the user of showing appexec target executable > as symlink target. But I am uncertain about the effect on code. > > One example: Any app that is able to archive/copy posix symlinks will > convert the appexec to a symlink and silently drop the appexec data. Whether > this is a significant issue depends on if most/all relevent store apps > function the same when the executable is exec-ed directly vs via the appexec > link. You won't get a sufficent, POSIX-like handling one way or the other. Handling them as files will not allow correct archiving either, because the reparse point data required to restore them correctly to work in Windows won't be archived anyway. There's only so much we can do in the Windows environment. ¯\_(ツ)_/¯ I'd just like to postpone this patch to get 3.2.0 out. I'll add that patch to the inevitable 3.2.1 release, given the massive testing of 3.2.0-0.1 on the Cygwin ML... Thanks, Corinna
Re: [PATCH] Cygwin: pty: Transfer input only if the stdin is a pty.
On Mar 9 12:23, Takashi Yano via Cygwin-patches wrote: > - The commit 12325677f73a did not fix enough. With this patch, more > transfer_input() calls are skipped if stdin is redirected or piped. > --- > winsup/cygwin/fhandler_tty.cc | 10 -- > winsup/cygwin/spawn.cc| 9 +++-- > 2 files changed, 15 insertions(+), 4 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Transfer input for native app only if the stdin is pcon.
On Mar 9 12:22, Takashi Yano via Cygwin-patches wrote: > On Mon, 8 Mar 2021 21:52:37 +0100 > Corinna Vinschen wrote: > > On Mar 9 00:48, Takashi Yano via Cygwin-patches wrote: > > > On Mon, 8 Mar 2021 16:32:16 +0100 > > > Corinna Vinschen wrote: > > > > Hi Takashi, > > > > > > > > On Mar 8 23:55, Takashi Yano via Cygwin-patches wrote: > > > > > - Currently, transfer input is triggered even if the stdin of native > > > > > app is not a pseudo console. With this patch it is triggered only > > > > > if the stdin is a pseudo console. > > > > > > > > do you have more patches in the loop? I wonder if I should really start > > > > the test release cycle for 3.2.0 or if I should wait a bit...? > > > > > > I'm sorry to submit patches one after another. However, > > > I think this should be the last one. Please go ahead. > > > > Great, thanks! > > I am very sorry but I would like to submit just one more patch. > I apologize to overturn the previous statement... No worries :) Corinna
Re: [PATCH] winsup/doc/dll.xml: update MinGW/.org to MinGW-w64/.org
On Mar 8 15:20, Ken Brown via Cygwin-patches wrote: > On 3/8/2021 2:09 PM, Achim Gratz wrote: > > Brian Inglis writes: > > > It's normally a merge conflict which will not be satisfied by regular > > > commands to restore the working files to upstream. > > > > So you're pulling on an unclean work tree? That's a no-no, either keep > > your changes on a separate branch (that you can rebase or merge later) > > or stash them away for the pull. > > > > As Corinna said, if you're prepared to lose any local changes then > > > > git reset --hard > > > > will do that. But you should be sure you really didn't want any of your > > unfinished business around any more. > > If the unfinished business consists of local commits that haven't yet been > applied upstream, then I typically do the following: > > git fetch # Find out if upstream has changed since my last pull. If so... > git format-patch -n # save n local commits > git reset --hard origin/master > git am 00* # reapply my local commits I'm doing this a bit differently: git fetch git rebase -i origin/master I like git rebase, it's a very nifty tool, especially using the interactive mode. Corinna
Re: [PATCH 0/2] cygwin-htdocs: update MinGW references
On Mar 8 09:17, Brian Inglis wrote: > update MinGW.org references and update package categories to remove MinGW > > Brian Inglis (2): > cygwin-htdocs/links.html: update MinGW.org reference > cygwin-htdocs/packaging-hint-files.html: update categories > > links.html| 4 ++-- > packaging-hint-files.html | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > -- > 2.30.1 Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Transfer input for native app only if the stdin is pcon.
On Mar 9 00:48, Takashi Yano via Cygwin-patches wrote: > On Mon, 8 Mar 2021 16:32:16 +0100 > Corinna Vinschen wrote: > > Hi Takashi, > > > > On Mar 8 23:55, Takashi Yano via Cygwin-patches wrote: > > > - Currently, transfer input is triggered even if the stdin of native > > > app is not a pseudo console. With this patch it is triggered only > > > if the stdin is a pseudo console. > > > > do you have more patches in the loop? I wonder if I should really start > > the test release cycle for 3.2.0 or if I should wait a bit...? > > I'm sorry to submit patches one after another. However, > I think this should be the last one. Please go ahead. Great, thanks! Corinna
Re: [PATCH] Cygwin: pty: Transfer input for native app only if the stdin is pcon.
On Mar 8 23:55, Takashi Yano via Cygwin-patches wrote: > - Currently, transfer input is triggered even if the stdin of native > app is not a pseudo console. With this patch it is triggered only > if the stdin is a pseudo console. > --- > winsup/cygwin/fhandler_tty.cc | 18 ++ > 1 file changed, 10 insertions(+), 8 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Transfer input for native app only if the stdin is pcon.
Hi Takashi, On Mar 8 23:55, Takashi Yano via Cygwin-patches wrote: > - Currently, transfer input is triggered even if the stdin of native > app is not a pseudo console. With this patch it is triggered only > if the stdin is a pseudo console. do you have more patches in the loop? I wonder if I should really start the test release cycle for 3.2.0 or if I should wait a bit...? Thanks, Corinna
Re: [PATCH] Cygwin: pty: Attach to stub process when non-cygwin app inherits pcon.
On Mar 8 22:14, Takashi Yano via Cygwin-patches wrote: > - If two non-cygwin apps are started simultaneously, attaching to > pseudo console sometimes fails. This is because the second app > trys to attach to the process not started yet. This patch avoids > the issue by attaching to the stub process rather than the other > non-cygwin app. > --- > winsup/cygwin/fhandler_tty.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc > index 4358bceec..3bfc8c0c8 100644 > --- a/winsup/cygwin/fhandler_tty.cc > +++ b/winsup/cygwin/fhandler_tty.cc > @@ -3104,7 +3104,7 @@ fhandler_pty_slave::setup_pseudoconsole (bool nopcon) > 0, TRUE, DUPLICATE_SAME_ACCESS); >CloseHandle (pcon_owner); >FreeConsole (); > - AttachConsole (p->dwProcessId); > + AttachConsole (p->exec_dwProcessId); >goto skip_create; > } > > -- > 2.30.1 Pushed. Thanks, Corinna
Re: [PATCH] winsup/doc/dll.xml: update MinGW/.org to MinGW-w64/.org
On Mar 7 13:26, Brian Inglis wrote: > On 2021-03-07 12:15, Jon Turney wrote: > > On 07/03/2021 16:31, Brian Inglis wrote: > > > --- > > > winsup/doc/dll.xml | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > I don't think the link here actually has much value, and would be > > inclined to drop it, as far as I can tell it's just giving that as an > > example of a toolchain which produces 'lib'-prefixed DLLs. > > > > Also, reading the whole page, the section "Linking against DLLs" needs > > updating since GNU ld has had the ability to link directly against DLLs > > (automatically generating the necessary import stubs) for a number of > > years. > > > > Also, there are other mentions of MinGW.org on the cygwin website (e.g. > > https://cygwin.com/links.html) which also need updating, if that URL is > > no longer valid. > > I checked the tree and Corinna cleaned up some a few years ago. > > I already checked winsup/doc/ and there were no other substantive uses of > MinGW unless you would prefer *ALL* mentions of MinGW be suffixed with -w64. > > I did not look closely at cygwin-htdocs, as git complained when I tried to > update, so I wiped that repo, If git complained, your repo was just not in the latets state. Maybe just using `git reset --hard origin/master' would have fixed it. Corinna
Re: [PATCH] winsup/doc/dll.xml: update MinGW/.org to MinGW-w64/.org
On Mar 7 19:15, Jon Turney wrote: > On 07/03/2021 16:31, Brian Inglis wrote: > > --- > > winsup/doc/dll.xml | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > I don't think the link here actually has much value, and would be inclined > to drop it, as far as I can tell it's just giving that as an example of a > toolchain which produces 'lib'-prefixed DLLs. However, telling people we use cyg instead of lib for $reason, and then not pointing to at least one project using this prefix seems puzzeling. Given that Windows doesn't use a prefix at all, the URL to mingw-w64 might help, doesn't it? > Also, reading the whole page, the section "Linking against DLLs" needs > updating since GNU ld has had the ability to link directly against DLLs > (automatically generating the necessary import stubs) for a number of years. What do you suggest? Shall we just remove the section entirely? > Also, there are other mentions of MinGW.org on the cygwin website (e.g. > https://cygwin.com/links.html) which also need updating, if that URL is no > longer valid. Yeah, we should remove them or move them to mingw-w64, too. Thanks, Corinna
Re: [PATCH] Cygwin: console, pty: Stop ignoring Ctrl-C by IGNBRK.
On Mar 7 10:41, Takashi Yano via Cygwin-patches wrote: > - Perhaps current code misunderstand meaning of the IGNBRK. As far > as I investigated, IGNBRK is concerned with break signal in serial > port but there is no evidence that it has effect to ignore Ctrl-C. > This patch stops ignoring Ctrl-C by IGNBRK for non-cygwin apps. > --- > winsup/cygwin/fhandler_console.cc | 2 +- > winsup/cygwin/fhandler_tty.cc | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Discard input already accepted on interrupt.
On Mar 5 18:01, Takashi Yano via Cygwin-patches wrote: > - Currently, input already accepted is not discarded on interrupt > by VINTR, VQUIT and VSUSP keys. This patch fixes the issue. > --- > winsup/cygwin/fhandler.h | 2 ++ > winsup/cygwin/fhandler_termios.cc | 5 - > winsup/cygwin/fhandler_tty.cc | 23 +++ > winsup/cygwin/tty.cc | 1 + > winsup/cygwin/tty.h | 1 + > 5 files changed, 31 insertions(+), 1 deletion(-) Pushed. Thanks, Corinna
Re: [PATCH v2] Cygwin: console: Fix restoring console mode failure.
On Mar 4 19:11, Takashi Yano via Cygwin-patches wrote: > - Restoring console mode fails in the following scenario. >1) Start cygwin shell in command prompt. >2) Run 'exec chcp.com'. > This patch fixes the issue. > --- > winsup/cygwin/fhandler.h | 1 + > winsup/cygwin/spawn.cc | 14 ++ > 2 files changed, 11 insertions(+), 4 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Fix a race issue in startup of pseudo console.
On Mar 4 17:56, Takashi Yano via Cygwin-patches wrote: > - If two non-cygwin apps are started simultaneously and this is the > first execution of non-cygwin apps in the pty, these occasionally > hang up. The cause is the race issue between term_has_pcon_cap(), > reset_switch_to_pcon() and setup_pseudoconsole(). This patch fixes > the issue. > --- > winsup/cygwin/fhandler_tty.cc | 16 > 1 file changed, 12 insertions(+), 4 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH] cygwin-htdocs/lists.html: add note about attachment size limits
On Mar 3 20:55, Brian Inglis wrote: > committer please adjust based on actual size limits if different: > (256KB - 8KB email text)/1.37 overhead ~ 180KB > 180KB * 1.37 overhead + 8KB email text ~ 256KB > --- > lists.html | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/lists.html b/lists.html > index fb784f8d2732..a3beda2d64e0 100755 > --- a/lists.html > +++ b/lists.html > @@ -42,7 +42,14 @@ answer. > > > > -None of the below lists accept href="https://sourceware.org/lists.html#html-mail;>html mail. Use plain > text only. > +None of the below lists accept > +https://sourceware.org/lists.html#html-mail;>html mail. > +Use plain text only. > +If you include attachments, please try to ensure they are in plain text, > +and limit them to about 180KB, as with encoding and email overhead, > +any larger will exceed the size limits for emails to these lists. > + > + > > Please do not feed the spammers by href="acronyms/#PCYMTNQREAIYR">including raw email addresses in the body > of your message. > Pushed. Thanks, Corinna
Re: [PATCH 0/7] Fix some system calls on sockets
On Feb 25 17:48, Ken Brown via Cygwin-patches wrote: > Several of the fhandler_socket_local and fhandler_socket_unix methods > that support system calls are written as though they are operating on > socket files unless the socket is an abstract socket. This patchset > (except for the last patch) attempts to fix this by checking whether > the fhandler is associated with a socket file. If not, we call an > fhandler_socket_wsock or fhandler_socket method instead of an > fhandler_disk_file method. > > The last patch is just a code simplification that arose while I was > working on fhandler_socket_local::link. > > Ken Brown (7): > Cygwin: fix fstat on sockets that are not socket files > Cygwin: fix fstatvfs on sockets that are not socket files > Cygwin: fix fchmod on sockets that are not socket files > Cygwin: fix fchown on sockets that are not socket files > Cygwin: fix facl on sockets that are not socket files > Cygwin: fix linkat(2) on sockets that are not socket files > Cygwin: simplify linkat with AT_EMPTY_PATH > > winsup/cygwin/fhandler_socket_local.cc | 39 +- > winsup/cygwin/fhandler_socket_unix.cc | 56 -- > winsup/cygwin/syscalls.cc | 24 +++ > 3 files changed, 81 insertions(+), 38 deletions(-) > > -- > 2.30.0 LGTM, please push. Thanks, Corinna
Re: [PATCH] Cygwin: AF_UNIX: allow opening with the O_PATH flag
On Feb 23 12:44, Ken Brown via Cygwin-patches wrote: > This was done for the fhandler_socket_local class in commits > 3a2191653a, 141437d374, and 477121317d, but the fhandler_socket_unix > class was overlooked. > --- > winsup/cygwin/fhandler.h | 1 + > winsup/cygwin/fhandler_socket_unix.cc | 24 > 2 files changed, 25 insertions(+) LGTM. Thanks, Corinna
Re: [PATCH 0/1] Fix facl on files opened with O_PATH
On Feb 23 17:49, Ken Brown via Cygwin-patches wrote:
> I'm not sure if this patch is right. Should facl fail on all commands
> or just on SETACL? If the command is GETACL, for example, should this
> fail like fgetxattr(2) or should it succeed like fstat(2)?
>
> Cygwin may be the only platform that supports both facl(2) and O_PATH,
> so I guess we're on our own here.
Not entirely. facl is also the underlying function for the POSIX ACL
calls, deprecated but still supported by Linux. I. e., on a file system
supporting ACLs (xfs, ext4, etc), this needs testing:
int fd = open (..., O_PATH);
if (fd < 0)
perror ("open");
else if (acl_get_fd (fd) != NULL)
printf ("acl_get_fd works with O_PATH\n");
else
perror ("acl_get_fd");
I just did that and it turns out that the above code returns with
acl_get_fd: Bad file descriptor
At first I was actually a bit surprised. I thought fetching ACLs is
along the lines of fstat, but on second though it's not. ACLs are
stored as extended attributes and given that fgetxattr is supposed to
fail with EBADF, it's logical that acl_get_fd fails with EBADF as well.
qed
So your patch looks good.
Thanks,
Corinna
Re: [PATCH] index.html: update Cygwin home page to show version 3.1.7
On Feb 23 13:48, Brian Inglis wrote: > --- > index.html | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/index.html b/index.html > index 037a9d87c968..7aafecea8c2e 100755 > --- a/index.html > +++ b/index.html > @@ -50,7 +50,7 @@ > > > The most recent version of the Cygwin DLL is > - href="https://cygwin.com/pipermail/cygwin-announce/2020-July/009605.html;>3.1.6. > + href="https://cygwin.com/pipermail/cygwin-announce/2020-August/009678.html;>3.1.7. > > >Installing Cygwin Oops. Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: console: Prevent NULL pointer access in close().
On Feb 22 22:30, Takashi Yano via Cygwin-patches wrote:
> - There seems to be a case that shared_console_info is not set yet
> when close() is called. This patch adds guard for such case.
> ---
> winsup/cygwin/fhandler_console.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/fhandler_console.cc
> b/winsup/cygwin/fhandler_console.cc
> index 6ded9eabf..96a8729e8 100644
> --- a/winsup/cygwin/fhandler_console.cc
> +++ b/winsup/cygwin/fhandler_console.cc
> @@ -1393,7 +1393,7 @@ fhandler_console::close ()
>
>release_output_mutex ();
>
> - if (con.owner == myself->pid)
> + if (shared_console_info && con.owner == myself->pid)
> {
>char name[MAX_PATH];
>shared_name (name, CONS_THREAD_SYNC, get_minor ());
> --
> 2.30.0
Pushed.
Thanks,
Corinna
Re: [PATCH] Cygwin: pty: Fix segfault caused when tcflush() is called.
On Feb 22 22:28, Takashi Yano via Cygwin-patches wrote: > On Mon, 22 Feb 2021 20:41:00 +0900 > Takashi Yano wrote: > > On Mon, 22 Feb 2021 10:51:19 +0100 > > Corinna Vinschen wrote: > > > So, what do you think is the state of the console code, Takashi? > > > Shall we start a release cycle next week? > > > > I think all the fixes and improvements that come to mind at this > > point have been completed. As for releasing, I believe I've done > > enough testing, but honestly I'm not without anxiety because total > > amount of changes for pty and console code is relatively large > > since the beginning of this year. > > Sure enough, I found a problem in the console code... :) > I will submit a patch for that. No worries, Corinna
Re: [PATCH] Cygwin: pty: Fix segfault caused when tcflush() is called.
On Feb 22 20:41, Takashi Yano via Cygwin-patches wrote: > On Mon, 22 Feb 2021 10:51:19 +0100 > Corinna Vinschen wrote: > > So, what do you think is the state of the console code, Takashi? > > Shall we start a release cycle next week? > > I think all the fixes and improvements that come to mind at this > point have been completed. As for releasing, I believe I've done > enough testing, but honestly I'm not without anxiety because total > amount of changes for pty and console code is relatively large > since the beginning of this year. > > On the other hand, I also want people to use new features as soon > as possible. Then let's do it. I'll start with the usual test releases... Thanks, Corinna
Re: [PATCH] Cygwin: pty: Fix segfault caused when tcflush() is called.
On Feb 21 07:45, Takashi Yano via Cygwin-patches wrote:
> - After commit 253352e796ff9ec9a447e5375f5bc3e2b92b5293, mc (midnight
> commander) crashes with segfault if the shell is bash. This is due
> to NULL pointer access in read(). This patch fixes the issue.
> Addresses::
> https://cygwin.com/pipermail/cygwin/2021-February/247870.html
> ---
> winsup/cygwin/fhandler_tty.cc | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index d30041af1..3fcaa8277 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -1474,8 +1474,11 @@ wait_retry:
> out:
>termios_printf ("%d = read(%p, %lu)", totalread, ptr, len);
>len = (size_t) totalread;
> - bool saw_eol = totalread > 0 && strchr ("\r\n", ptr0[totalread -1]);
> - mask_switch_to_pcon_in (false, saw_eol);
> + if (ptr0)
> +{ /* Not tcflush() */
> + bool saw_eol = totalread > 0 && strchr ("\r\n", ptr0[totalread -1]);
> + mask_switch_to_pcon_in (false, saw_eol);
> +}
> }
>
> int
> --
> 2.30.0
Pushed.
So, what do you think is the state of the console code, Takashi?
Shall we start a release cycle next week?
Thanks,
Corinna
Re: [PATCH 0/2] Cygwin: pty, console: Make FLUSHO and Ctrl-O work.
On Feb 19 17:44, Takashi Yano via Cygwin-patches wrote: > - With these patches, FLUSHO and Ctrl-O (VDISCARD) get working. > > Takashi Yano (2): > Cygwin: pty: Make FLUSHO and Ctrl-O work. > Cygwin: console: Add support for FLUSHO and Ctrl-O. > > winsup/cygwin/fhandler.h | 1 + > winsup/cygwin/fhandler_console.cc | 11 +++ > winsup/cygwin/fhandler_tty.cc | 17 +++-- > winsup/cygwin/select.cc | 5 + > winsup/cygwin/tty.cc | 1 + > winsup/cygwin/tty.h | 1 + > 6 files changed, 30 insertions(+), 6 deletions(-) > > -- > 2.30.0 Pushed. Thanks, Corinna
Re: [PATCH 0/3] Fix fstat on FIFOs, part 2
On Feb 18 12:13, Ken Brown via Cygwin-patches wrote: > The first patch fixes a bug, in which fstat on FIFOs sometimes used > pipe handles instead of file handles. > > The second and third patches should improve the efficiency of fstat > and open on FIFOs. > > Ken Brown (3): > Cygwin: define fhandler_fifo::fstat > Cygwin: fstat_helper: always use handle in call to get_file_attribute > Cygwin: FIFO: temporarily keep a conv_handle in syscalls.cc:open > > winsup/cygwin/fhandler.h| 1 + > winsup/cygwin/fhandler_disk_file.cc | 3 +-- > winsup/cygwin/fhandler_fifo.cc | 23 +++ > winsup/cygwin/syscalls.cc | 22 +++--- > 4 files changed, 44 insertions(+), 5 deletions(-) > > -- > 2.30.0 LGTM, please push. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Make tty setting NOFLSH work.
On Feb 18 18:07, Takashi Yano via Cygwin-patches wrote:
> - With this patch, "stty noflsh" gets working in pty.
> ---
> winsup/cygwin/fhandler_termios.cc | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/fhandler_termios.cc
> b/winsup/cygwin/fhandler_termios.cc
> index e8daf946b..ae35fe894 100644
> --- a/winsup/cygwin/fhandler_termios.cc
> +++ b/winsup/cygwin/fhandler_termios.cc
> @@ -332,7 +332,8 @@ fhandler_termios::line_edit (const char *rptr, size_t
> nread, termios& ti,
> goto not_a_sig;
>
> termios_printf ("got interrupt %d, sending signal %d", c, sig);
> - eat_readahead (-1);
> + if (!(ti.c_lflag & NOFLSH))
> + eat_readahead (-1);
> release_input_mutex_if_necessary ();
> tc ()->kill_pgrp (sig);
> acquire_input_mutex_if_necessary (INFINITE);
> --
> 2.30.0
Pushed.
Thanks,
Corinna
Re: [PATCH] Cygwin: pty: Reflect tty settings to pseudo console mode.
On Feb 18 18:05, Takashi Yano via Cygwin-patches wrote:
> - With this patch, tty setting such as echo, icanon, isig and onlcr
> are reflected to pseudo console mode.
> ---
> winsup/cygwin/fhandler_tty.cc | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index 5afede859..e4c35ea41 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -3261,6 +3261,33 @@ skip_create:
>if (get_ttyp ()->previous_output_code_page)
> SetConsoleOutputCP (get_ttyp ()->previous_output_code_page);
>
> + do
> +{
> + termios = get_ttyp ()->ti;
> + DWORD mode;
> + /* Set input mode */
> + GetConsoleMode (hpConIn, );
> + mode &= ~(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT |
> ENABLE_PROCESSED_INPUT);
> + if (t.c_lflag & ECHO)
> + mode |= ENABLE_ECHO_INPUT;
> + if (t.c_lflag & ICANON)
> + mode |= ENABLE_LINE_INPUT;
> + if (mode & ENABLE_ECHO_INPUT && !(mode & ENABLE_LINE_INPUT))
> + /* This is illegal, so turn off the echo here, and fake it
> +when we read the characters */
> + mode &= ~ENABLE_ECHO_INPUT;
> + if ((t.c_lflag & ISIG) && !(t.c_iflag & IGNBRK))
> + mode |= ENABLE_PROCESSED_INPUT;
> + SetConsoleMode (hpConIn, mode);
> + /* Set output mode */
> + GetConsoleMode (hpConOut, );
> + mode &= ~DISABLE_NEWLINE_AUTO_RETURN;
> + if (!(t.c_oflag & OPOST) || !(t.c_oflag & ONLCR))
> + mode |= DISABLE_NEWLINE_AUTO_RETURN;
> + SetConsoleMode (hpConOut, mode);
> +}
> + while (false);
> +
>return true;
>
> cleanup_pcon_in:
> --
> 2.30.0
Pushed.
Thanks,
Corinna
Re: [PATCH] Cygwin: Add console fix regarding Ctrl-Z etc. to release notes.
On Feb 18 18:02, Takashi Yano via Cygwin-patches wrote: > --- > winsup/cygwin/release/3.2.0 | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/winsup/cygwin/release/3.2.0 b/winsup/cygwin/release/3.2.0 > index d02d16863..d69ed446c 100644 > --- a/winsup/cygwin/release/3.2.0 > +++ b/winsup/cygwin/release/3.2.0 > @@ -9,6 +9,11 @@ What's new: >thrd_detach, thrd_equal, thrd_exit, thrd_join, thrd_sleep, thrd_yield, >tss_create, tss_delete, tss_get, tss_set. > > +- In cygwin console, new thread which handles special keys/signals such > + as Ctrl-Z (VSUSP), Ctrl-\ (VQUIT), Ctrl-S (VSTOP), Ctrl-Q (VSTART) and > + SIGWINCH has been intrudocued. There have been a long standing issue > + that these keys/signals are handled only when app calls read() or > + select(). Now, these work even if app does not call read() or select(). > > What changed: > - > -- > 2.30.0 Pushed with a minor typo fix and I dup'ed this text to doc/new-features.xml Thanks, Corinna
Re: [PATCH 0/2] Console fixes for Win7.
On Feb 18 18:01, Takashi Yano via Cygwin-patches wrote: > Takashi Yano (2): > Cygwin: console: Fix SIGWINCH handling in Win7. > Cygwin: console: Fix handling of Ctrl-S in Win7. > > winsup/cygwin/fhandler.h | 9 +- > winsup/cygwin/fhandler_console.cc | 306 -- > winsup/cygwin/select.cc | 4 +- > winsup/cygwin/spawn.cc| 32 ++-- > winsup/cygwin/tty.h | 8 +- > 5 files changed, 113 insertions(+), 246 deletions(-) > > -- > 2.30.0 Pushed. Thanks, Corinna
Re: [PATCH v2 0/2] cpuinfo: fix check; add AVX features; move SME, SEV/_ES features
On Feb 17 09:28, Brian Inglis wrote: > Brian Inglis (2): > fix check for cpuid 0x8007 support > add AVX features; move SME, SEV/_ES features > > winsup/cygwin/fhandler_proc.cc | 46 ++ > 1 file changed, 24 insertions(+), 22 deletions(-) > > -- > 2.30.0 Pushed. Thanks, Corinna
Re: [PATCH] cpuinfo: fix check; add AVX features; move SME, SEV/_ES features
Hi Brian, On Feb 16 09:00, Brian Inglis wrote: > > fix cpuid 0x8007 supported check; > Linux 5.11 Valentine's Day Edition added features and changes: > add Intel 0x0007 EDX:23 avx512_fp16 and 0x0007:1 EAX:4 avx_vnni; > group scattered AMD 0x801f EAX Secure Mem/Encrypted Virt features at end: > 0 sme, 1 sev, 3 sev_es (more to come not yet displayed) > --- > winsup/cygwin/fhandler_proc.cc | 46 ++ > 1 file changed, 24 insertions(+), 22 deletions(-) > > diff --git a/winsup/cygwin/fhandler_proc.cc b/winsup/cygwin/fhandler_proc.cc > index 8e23c0609485..501c157daae5 100644 > --- a/winsup/cygwin/fhandler_proc.cc > +++ b/winsup/cygwin/fhandler_proc.cc > @@ -1293,7 +1293,7 @@ format_proc_cpuinfo (void *, char *) > > /* features scattered in various CPUID levels. */ >/* cpuid 0x8007 edx */ > - if (maxf >= 0x0007) > + if (maxe >= 0x8007) Maybe I'm a stickler here, but I think it would be nice to put this bugfix into its own patch, prior to the patch adding the 5.11 features. Thanks, Corinna
Re: [PATCH v2] Cygwin: console: Introduce new thread which handles input signal.
On Feb 16 20:37, Takashi Yano via Cygwin-patches wrote: > - Currently, Ctrl-Z, Ctrl-\ and SIGWINCH does not work in console > if the process does not call read() or select(). This is because > these are processed in process_input_message() which is called > from read() or select(). This is a long standing issue of console. > Addresses: > https://cygwin.com/pipermail/cygwin/2020-May/244898.html > https://cygwin.com/pipermail/cygwin/2021-February/247779.html > > With this patch, new thread which handles only input signals is > introduced so that Crtl-Z, etc. work without calling read() or > select(). Ctrl-S and Ctrl-Q are also handled in this thread. > --- > winsup/cygwin/exceptions.cc | 1 + > winsup/cygwin/fhandler.h | 5 +- > winsup/cygwin/fhandler_console.cc | 177 +- > 3 files changed, 181 insertions(+), 2 deletions(-) Pushed. This is great! Can you please add an entry to the release docs? Thanks, Corinna
Re: [PATCH v2] winsup/doc/posix.xml: add note for getrlimit, setrlimit, xrefs to notes
On Feb 16 10:00, Brian Inglis wrote: > On 2021-02-16 09:51, Brian Inglis wrote: > > On 2021-02-16 04:30, Corinna Vinschen via Cygwin-patches wrote: > > > On Feb 15 15:35, Brian Inglis wrote: > > > > change notes to see "Implementation Notes" to xref to std-notes; > > > > add xref to std-notes to getrlimit, setrlimit; > > > > add note to document limitations of getrlimit, setrlimit resources > > > > support > > > > --- > > > > winsup/doc/posix.xml | 101 --- > > > > 1 file changed, 57 insertions(+), 44 deletions(-) > > > > > Pushed with a change: > > > (see chapter "Implementation Notes") > > > --> > > > (see chapter "Implementation Notes") > > > The reason is how xref is handled when creating html docs. The result > > > of an xref is always 'the section called "..."'. With the above change, > > > the text works (albeit differently) in html and info file. > > > > Cheers, thanks, I'll bear that in mind in future, and read the generated > > output more carefully. > > > > I'm not seeing .info generated with Note:... links, is that okay? db2x_docbook2texi from docbook2X is used for that. > > Also ...api.pdf is no longer being regenerated, so what have I lost or am > > missing? xmlto is doing this stuff, maybe the pdf option requires another package? > > I have the following doc tools (and others): > > > > $ apt l asciidoc dblatex poppler\$ xmlto > > asciidoc 8.6.9-1 x86_64 [installed, manual] > > dblatex 0.3.10-1 x86_64 [installed, automatic] > > poppler 21.01.0-1 x86_64 [installed, manual] > > xmlto 0.0.28-1 x86_64 [installed, automatic] > > Also as documented plus other dependencies: > > $ apt l build-docbook-catalog docbook-sgml45 docbook-utils docbook-xml45 \ > docbook-xsl docbook2X > build-docbook-catalog 1.5-2 x86_64 [installed, automatic] > docbook-sgml45 4.5-1 x86_64 [installed, automatic] > docbook-utils 0.6.14-2 x86_64 [installed, manual] > docbook-xml45 4.5-1 x86_64 [installed, manual] > docbook-xsl 1.77.1-1 x86_64 [installed, automatic] > docbook2X 0.8.8-1 x86_64 [installed, manual] > > > What else is needed? Good question. I'm running that on Fedora with the following docbook packages installed: docbook-dtds docbook-style-xsl docbook-style-dsssl docbook-utils docbook2X xmlto adds a dependency to flex. docbook2X depends on texinfo, texinfo comes with a number of packages on the far side of infinity. Jon? Any idea? Thanks, Corinna
Re: [PATCH v2] winsup/doc/posix.xml: add note for getrlimit, setrlimit, xrefs to notes
Hi Brian, On Feb 15 15:35, Brian Inglis wrote: > > change notes to see "Implementation Notes" to xref to std-notes; > add xref to std-notes to getrlimit, setrlimit; > add note to document limitations of getrlimit, setrlimit resources support > --- > winsup/doc/posix.xml | 101 --- > 1 file changed, 57 insertions(+), 44 deletions(-) Pushed with a change: (see chapter "Implementation Notes") --> (see chapter "Implementation Notes") The reason is how xref is handled when creating html docs. The result of an xref is always 'the section called "..."'. With the above change, the text works (albeit differently) in html and info file. Thanks, Corinna
Re: [PATCH v2] Cygwin: console: Abort read() on signal if SA_RESTART is not set.
On Feb 15 22:08, Takashi Yano via Cygwin-patches wrote: > On Mon, 15 Feb 2021 13:04:41 +0100 > Corinna Vinschen wrote: > > On Feb 14 18:42, Takashi Yano via Cygwin-patches wrote: > > > - Currently, console read() keeps reading after SIGWINCH is sent > > > even if SA_RESTART flag is not set. With this patch, read() > > > returns EINTR on SIGWINCH if SA_RESTART flag is not set. > > > The same problem for SIGQUIT and SIGTSTP has also been fixed. > > > --- > > > winsup/cygwin/fhandler_console.cc | 7 +++ > > > winsup/cygwin/fhandler_termios.cc | 1 + > > > winsup/cygwin/tty.cc | 1 + > > > winsup/cygwin/tty.h | 1 + > > > 4 files changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/winsup/cygwin/fhandler_console.cc > > > b/winsup/cygwin/fhandler_console.cc > > > index 3c0783575..78af6cf2b 100644 > > > --- a/winsup/cygwin/fhandler_console.cc > > > +++ b/winsup/cygwin/fhandler_console.cc > > > @@ -586,12 +586,11 @@ wait_retry: > > > case input_ok: /* input ready */ > > > break; > > > case input_signalled: /* signalled */ > > > - release_input_mutex (); > > > - /* The signal will be handled by cygwait() above. */ > > > - continue; > > > case input_winch: > > > release_input_mutex (); > > > - continue; > > > + if (global_sigs[get_ttyp ()->last_sig].sa_flags & SA_RESTART) > > > > Shouldn't this check for last_sig != 0 first? > > This code is reached only after SIGINT, SIGTSTP, SIGQUIT (case > input_signalled) or SIGWINCH (case input_winch) has been sent. > Therefore, last_sig should be one of them here. Thanks, pushed. Corinna
Re: [PATCH] Cygwin: pty: Fix a bug in input transfer for GDB.
On Feb 15 03:47, Takashi Yano via Cygwin-patches wrote:
> - With this patch, not only NL but also CR is treated as a line end
> in the code checking if input transfer is necessary.
> ---
> winsup/cygwin/fhandler_tty.cc | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index f6eb3ae4d..5afede859 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -1181,7 +1181,7 @@ fhandler_pty_slave::mask_switch_to_pcon_in (bool mask,
> bool xfer)
>/* In GDB, transfer input based on setpgid() does not work because
> GDB may not set terminal process group properly. Therefore,
> transfer input here if isHybrid is set. */
> - if (get_ttyp ()->switch_to_pcon_in && !!masked != mask && xfer && isHybrid)
> + if (isHybrid && !!masked != mask && xfer)
> {
>if (mask && get_ttyp ()->pcon_input_state_eq (tty::to_nat))
> {
> @@ -1471,7 +1471,8 @@ wait_retry:
> out:
>termios_printf ("%d = read(%p, %lu)", totalread, ptr, len);
>len = (size_t) totalread;
> - mask_switch_to_pcon_in (false, totalread > 0 && ptr0[totalread - 1] ==
> '\n');
> + bool saw_eol = totalread > 0 && strchr ("\r\n", ptr0[totalread -1]);
> + mask_switch_to_pcon_in (false, saw_eol);
> }
>
> int
> --
> 2.30.0
Pushed.
Thanks,
Corinna
Re: [PATCH v2] Cygwin: console: Abort read() on signal if SA_RESTART is not set.
Hi Takashi, On Feb 14 18:42, Takashi Yano via Cygwin-patches wrote: > - Currently, console read() keeps reading after SIGWINCH is sent > even if SA_RESTART flag is not set. With this patch, read() > returns EINTR on SIGWINCH if SA_RESTART flag is not set. > The same problem for SIGQUIT and SIGTSTP has also been fixed. > --- > winsup/cygwin/fhandler_console.cc | 7 +++ > winsup/cygwin/fhandler_termios.cc | 1 + > winsup/cygwin/tty.cc | 1 + > winsup/cygwin/tty.h | 1 + > 4 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/winsup/cygwin/fhandler_console.cc > b/winsup/cygwin/fhandler_console.cc > index 3c0783575..78af6cf2b 100644 > --- a/winsup/cygwin/fhandler_console.cc > +++ b/winsup/cygwin/fhandler_console.cc > @@ -586,12 +586,11 @@ wait_retry: > case input_ok: /* input ready */ > break; > case input_signalled: /* signalled */ > - release_input_mutex (); > - /* The signal will be handled by cygwait() above. */ > - continue; > case input_winch: > release_input_mutex (); > - continue; > + if (global_sigs[get_ttyp ()->last_sig].sa_flags & SA_RESTART) Shouldn't this check for last_sig != 0 first? Thanks, Corinna
Re: [PATCH] winsup/doc/posix.xml: add note for getrlimit, setrlimit, links to notes
Hi Brian, On Feb 12 18:06, Brian Inglis wrote: > > change notes to see "Implementation Notes" to links to std-notes.html; > links work in html docs but appear as text in info docs; > add link to std-notes.html to getrlimit, setrlimit; > add note to document limitations of getrlimit, setrlimit resources support > --- > winsup/doc/posix.xml | 101 --- > 1 file changed, 57 insertions(+), 44 deletions(-) > Thanks for the patch, but... > diff --git a/winsup/doc/posix.xml b/winsup/doc/posix.xml > index 0669d07de890..71f0373940a5 100644 > --- a/winsup/doc/posix.xml > +++ b/winsup/doc/posix.xml > @@ -64,7 +64,7 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008). > atoi > atol > atoll > -basename (see chapter "Implementation Notes") > +basename (see chapter > "Implementation Notes") ...please use xref rather than ulink for cross refs within the same documentation, i.e. (see chapter "Implementation Notes") Unfortunately I just noticed in both cases, that a matching link is missing in cygwin-api.info afterwards. However, cross referencing works in cygwin-ug-net.info, afaics. Jon, any idea why this is? I don't see any difference in how the info files are created. Thanks, Corinna
Re: [PATCH v2] Cygwin: pty: Reduce unecessary input transfer.
On Feb 11 18:09, Takashi Yano via Cygwin-patches wrote: > - Currently, input transfer is performed every time one line is read(), > if the non-cygwin app is running in the background. With this patch, > transfer is triggered by setpgid() rather than read() so that the > unnecessary input transfer can be reduced much in that situation. > --- > winsup/cygwin/fhandler.h | 15 +- > winsup/cygwin/fhandler_tty.cc | 377 -- > winsup/cygwin/spawn.cc| 78 --- > winsup/cygwin/tty.cc | 89 > winsup/cygwin/tty.h | 16 +- > 5 files changed, 376 insertions(+), 199 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH v2] Cygwin: Have tmpfile(3) use O_TMPFILE
On Feb 10 22:53, Mark Geisert wrote:
> Per discussion on cygwin-developers, a Cygwin tmpfile(3) implementation
> has been added to syscalls.cc. This overrides the one supplied by
> newlib. Then the open(2) flag O_TMPFILE was added to the open call that
> tmpfile internally makes.
>
> This v2 patch removes O_CREAT from open() call as O_TMPFILE obviates it.
> Note that open() takes a directory's path but returns an fd to a file.
> ---
> winsup/cygwin/release/3.2.0 | 4
> winsup/cygwin/syscalls.cc | 17 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/winsup/cygwin/release/3.2.0 b/winsup/cygwin/release/3.2.0
> index f748a9bc8..d02d16863 100644
> --- a/winsup/cygwin/release/3.2.0
> +++ b/winsup/cygwin/release/3.2.0
> @@ -19,6 +19,10 @@ What changed:
>
> - A few FAQ updates.
>
> +- Have tmpfile(3) make use of Win32 FILE_ATTRIBUTE_TEMPORARY via open(2)
> + flag O_TMPFILE.
> + Addresses: https://cygwin.com/pipermail/cygwin/2021-January/247304.html
> +
>
> Bug Fixes
> -
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 52a020f07..4cda69033 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -5225,3 +5225,20 @@ pipe2 (int filedes[2], int mode)
>syscall_printf ("%R = pipe2([%d, %d], %y)", res, read, write, mode);
>return res;
> }
> +
> +extern "C" FILE *
> +tmpfile (void)
> +{
> + char *dir = getenv ("TMPDIR");
> + if (!dir)
> +dir = P_tmpdir;
> + int fd = open (dir, O_RDWR | O_BINARY | O_TMPFILE, S_IRUSR | S_IWUSR);
> + if (fd < 0)
> +return NULL;
> + FILE *fp = fdopen (fd, "wb+");
> + int e = errno;
> + if (!fp)
> +close (fd); // ..will remove tmp file
> + set_errno (e);
> + return fp;
> +}
The patch was missing the EXPORT_ALIAS for tmpfile64, as outlined in
https://cygwin.com/pipermail/cygwin-developers/2021-February/012039.html
I added this to the patch and pushed it.
Thanks,
Corinna
Re: [PATCH] Cygwin: Have tmpfile(3) use O_TMPFILE
On Feb 10 01:07, Mark Geisert wrote:
> Hi Corinna,
>
> Corinna Vinschen via Cygwin-patches wrote:
> > Hi Mark,
> >
> > On Feb 9 02:50, Mark Geisert wrote:
> > > Per discussion on cygwin-developers, a Cygwin tmpfile(3) implementation
> > > has been added to syscalls.cc. This overrides the one supplied by
> > > newlib. Then the open(2) flag O_TMPFILE was added to the open call that
> > > tmpfile internally makes.
> > > ---
> > > winsup/cygwin/release/3.2.0 | 4
> > > winsup/cygwin/syscalls.cc | 20
> > > 2 files changed, 24 insertions(+)
> > >
> > > diff --git a/winsup/cygwin/release/3.2.0 b/winsup/cygwin/release/3.2.0
> > > index f748a9bc8..d02d16863 100644
> > > --- a/winsup/cygwin/release/3.2.0
> > > +++ b/winsup/cygwin/release/3.2.0
> > > @@ -19,6 +19,10 @@ What changed:
> > > - A few FAQ updates.
> > > +- Have tmpfile(3) make use of Win32 FILE_ATTRIBUTE_TEMPORARY via open(2)
> > > + flag O_TMPFILE.
> > > + Addresses: https://cygwin.com/pipermail/cygwin/2021-January/247304.html
> > > +
> > > Bug Fixes
> > > -
> > > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> > > index 52a020f07..b79c1c7cd 100644
> > > --- a/winsup/cygwin/syscalls.cc
> > > +++ b/winsup/cygwin/syscalls.cc
> > > @@ -5225,3 +5225,23 @@ pipe2 (int filedes[2], int mode)
> > > syscall_printf ("%R = pipe2([%d, %d], %y)", res, read, write, mode);
> > > return res;
> > > }
> > > +
> > > +extern "C" FILE *
> > > +tmpfile (void)
> > > +{
> > > + char *dir = getenv ("TMPDIR");
> >
> > This isn't what Linux tmpfile does. Per the man page, it tries to
> > create the file in P_tmpdir first, and if that fails, it tries
> > "/tmp".
>
> Oops, I was following newlib's code here. I'll adjust this.
Oops, I didn't check with newlib, sorry. Hmm, so we're stuck with
either Linux-compat or backward-compat but not both. Decisions,
decisions...
Let's stick to backward-compat then, so no changes here.
> > > + if (!dir)
> > > +dir = P_tmpdir;
> > > + int fd = open (dir, O_RDWR | O_CREAT | O_BINARY | O_TMPFILE,
> >
> > You have to specify O_EXCL here. The idea is that this file cannot be
> > made permanent, and missing the O_EXCL flag allows exactly that. See
> > https://man7.org/linux/man-pages/man2/open.2.html, the lengthy
> > description in terms of O_TMPFILE.
>
> I started out with O_EXCL as you suggested, but found syscalls.cc:1504
> reporting EEXIST. Is there some clash there between fh->exists() and
> O_TMPFILE? Hmm.
You specify O_CREAT as well. O_TMPFILE replaces O_CREAT. The
combination O_CREAT|O_EXCL still checks for existence of the file, in
that case, the dir.
Thanks,
Corinna
Re: [PATCH 0/1] Fix fstat on FIFOs, part 1
On Feb 9 10:11, Ken Brown via Cygwin-patches wrote: > Commit 76dca77f04 had a careless blunder, so this patch reverts it. > > Nevertheless, fstat(2) can be made more efficient on FIFOs, and I'm > working on a separate patchset to do this right. It's worth doing, > because every call to open(2) on a FIFO leads to a call chain > > device_access_denied --> fhaccess --> fstat, > > and this is one of the cases where greater efficiency is possible. > > Ken Brown (1): > Revert "Cygwin: fstat_helper: always use handle in call to > get_file_attribute" > > winsup/cygwin/fhandler_disk_file.cc | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) Sure, go ahead. Thanks, Corinna
Re: [PATCH] Cygwin: Have tmpfile(3) use O_TMPFILE
Hi Mark,
On Feb 9 02:50, Mark Geisert wrote:
> Per discussion on cygwin-developers, a Cygwin tmpfile(3) implementation
> has been added to syscalls.cc. This overrides the one supplied by
> newlib. Then the open(2) flag O_TMPFILE was added to the open call that
> tmpfile internally makes.
> ---
> winsup/cygwin/release/3.2.0 | 4
> winsup/cygwin/syscalls.cc | 20
> 2 files changed, 24 insertions(+)
>
> diff --git a/winsup/cygwin/release/3.2.0 b/winsup/cygwin/release/3.2.0
> index f748a9bc8..d02d16863 100644
> --- a/winsup/cygwin/release/3.2.0
> +++ b/winsup/cygwin/release/3.2.0
> @@ -19,6 +19,10 @@ What changed:
>
> - A few FAQ updates.
>
> +- Have tmpfile(3) make use of Win32 FILE_ATTRIBUTE_TEMPORARY via open(2)
> + flag O_TMPFILE.
> + Addresses: https://cygwin.com/pipermail/cygwin/2021-January/247304.html
> +
>
> Bug Fixes
> -
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 52a020f07..b79c1c7cd 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -5225,3 +5225,23 @@ pipe2 (int filedes[2], int mode)
>syscall_printf ("%R = pipe2([%d, %d], %y)", res, read, write, mode);
>return res;
> }
> +
> +extern "C" FILE *
> +tmpfile (void)
> +{
> + char *dir = getenv ("TMPDIR");
This isn't what Linux tmpfile does. Per the man page, it tries to
create the file in P_tmpdir first, and if that fails, it tries
"/tmp".
> + if (!dir)
> +dir = P_tmpdir;
> + int fd = open (dir, O_RDWR | O_CREAT | O_BINARY | O_TMPFILE,
You have to specify O_EXCL here. The idea is that this file cannot be
made permanent, and missing the O_EXCL flag allows exactly that. See
https://man7.org/linux/man-pages/man2/open.2.html, the lengthy
description in terms of O_TMPFILE.
Thanks,
Corinna
Re: [PATCH 09/11] mount.cc: Implement poor-man's cache
On Feb 3 12:38, Ben wrote: > > > On 18-01-2021 12:51, Corinna Vinschen via Cygwin-patches wrote: > > Ok, so hash_prefix reduces the path to a drive letter or the UNC path > > prefix and hashes it. However, what about partitions mounted to a > > subdir of, say, drive C? In that case the hashing goes awry, because > > you're comparing with the hash of drive C while the path is actually > > pointing to another partition. > > > How can I mount a partition as a subdir of drive C? > For some reason I can't: > $ mount /cygdrive/e/Temp/dummy /cygdrive/c/Temp/dummy/dummyone > mount: /cygdrive/c/Temp/dummy/dummyone: Invalid argument I wasn't talking about Cygwin mount points, but rather about Windows mount points. Since Windows 2000 a partition can be mounted into a directory of another partition. Only drive C: (ignoring non-harddisks) has to be mounted with a drive letter, all others can be mounted just as on Unix. But, yeah, Cygwin also supports bind mounts. Here's an example from my /etc/fstab.d user mount file: //remote/cygwin-src /cygwin nfs binary 0 0 /cygwin/pub /home/pub nfs bind 0 0 Corinna
Re: [PATCH v2 4/8] syscalls.cc: Implement non-path_conv dependent _unlink_nt
On Feb 3 12:03, Ben wrote:
> On 26-01-2021 12:34, Corinna Vinschen via Cygwin-patches wrote:
> >> + static bool has_posix_unlink_semantics =
> >> + wincap.has_posix_unlink_semantics ();
> >> + static bool has_posix_unlink_semantics_with_ignore_readonly =
> >> + wincap.has_posix_unlink_semantics_with_ignore_readonly ();
> >
> > Did you mean `const' rather than `static', by any chance? Either way, I
> > don't think these local vars are required, given that the wincap
> > accessors are already marked as const. The compiler should know how to
> > opimize this sufficiently.
> >
> I do mean static.
> With each instantiation these are initialized to the wincap value.
> Then later on, we might disable the behavior if we encounter a driver
> which returns: STATUS_INVALID_PARAMETER
>
> Assuming most files will reside on the same fs, (ie through the same driver)
> this will save use from the system call which we know isn't supported.
But this is an invalid assumption. In fact, it only hold true for
`rm -r' scenarios, not for anything else. What about long running
processes unlinking files in various spots under /var, /tmp, etc.,
i. .e. services. Or what about mc?
Keep in mind that unlink/rmdir are system calls. Each an every call is
independent of each other. If you encounter two unlink calls, you don't
even know how much time has gone by between them. Even if they occur in
a short time frame, you still don't know if they are even remotely
related. For all you know, they could be called from different threads,
doing different things on different partitions.
As such, using a static state at this point and keeping it around for
later calls is a bug.
> >> + ULONG flags = FILE_OPEN_REPARSE_POINT | FILE_OPEN_FOR_BACKUP_INTENT
> >> + | FILE_DELETE_ON_CLOSE | eflags;
> >
> > This looks like a dangerous assumption. So far we don't open unknown
> > reparse points as reparse points deliberately. No one knows what a
> > unknown reparse point is good for or supposed to do, so we don't even
> > know if we are allowed to handle it analogue to a symlink.
> >
> When opening these, you are correct.
> However, when a request is made to delete a reparse point, it's safe
> - even for an unknown reparse point - to assume that it is the reparse point
> itself which is to be deleted. Ofcourse: That's my theory.
It's definitely a deviation from the previous behaviour and I'm not
exactly comfortable with it. The problem is that only a minor part of
the reparse point population is actually something akin to a symlink. I
don't see how it can be a safe bet to allow the user to remove an RP
with unknown and, perhaps, crucial functionality to some given product.
> > Consequentially we open unknown reparse points just as normal files, so
> > that the reparse point's automatisms may kick in. By omitting this
> > step, we're moving on thin ice.
> >
> This would mean an unknown reparse point can never be deleted.
> I'm just not sure if that's what we should want.
It's what we do right now. We're trying to handle all RP types known to
constitute some kind of symlink, and if we learn about the meaning of as
yet unknown RPs, and it *is* some kind of symlink, we can add it to the
list.
> >> +{
> >> + //Step 2
> >> + //Reopen with all sharing flags, will set delete flag ourselves.
> >> + access |= FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES;
> >> + flags &= ~FILE_DELETE_ON_CLOSE;
> >> + fstatus = NtOpenFile (, access, attr, ,
> >> FILE_SHARE_VALID_FLAGS, flags);
> >> + debug_printf ("NtOpenFile %S: %y", attr->ObjectName, fstatus);
> >> +
> >> + if (NT_SUCCESS (fstatus))
> >> +{
> >> + if (has_posix_unlink_semantics_with_ignore_readonly)
> >> +{
> >> + //Step 3
> >> + //Remove the file with POSIX unlink semantics, ignore
> >> readonly flags.
> >
> > No check for NTFS? Posix semantics are not supported on any other FS.
> > No check for remote? Just because you support POSIX semantics on
> > *this* machine, doesn't mean the remote machine supports it at all...
> >
> Indeed no checks.
> If the driver correctly returns STATUS_INVALID_PARAMETER we will not try
> again (by
> resetting the has_posix_unlink_semantics_with_ignore_readonly flag and then
> fallback to
> usual trickery. If the driver returns error (but not
> STATUS_INVALID_PARAMETER) that
> driver pays a single kernel call, which I deem acceptable.
That's back to the static var then, which isn't feasible. For non-NTFS
or remote FSes you will intro
Re: fhandler_serial.cc: MARK and SPACE parity for serial port
On Feb 1 22:26, Marek Smetana via Cygwin-patches wrote: > I'm Sorry, this is my first patch using the mailing list. No worries. Patch pushed. Thanks, Corinna
Re: [PATCH] CYGWIN: Fix resolver debugging output
On Feb 1 15:46, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin-patches wrote: > > Except, the value has no meaning for ipv6. > > It'll print all 0's :-) But: > > minires does not make use of the _ext field. It does use the conventional > nsaddr_list (which is IPv4), > but only if Windows native DNS API is not used: "osquery"(aka use_os)=0. > > For debugging purposes, that is enough and very convenient (yet the output > needed some tune-up, which I suggested in my patch). Ok. > But for practical purposes, only Windows API should be used in regular > applications (which is the default, anyways, since > /etc/resolv.conf is not routinely provided, so "osquery=1" implicitly). Yeah, I think so, too. Ideally we should have stripped out all code providing non-Windows means (i. e., /etc/resolv.conf support) back when this code was folded into Cygwin. It just doesn't make sense, at least not by default. > I'm not sure if improvements to use _ext by the minires own code would be any > beneficial. > > Having said that, replies should be made understood by the minires-if-os > shim code > (and I can provide a patch for that, too). That would be great! Thanks, Corinna
Re: [PATCH] CYGWIN: Fix resolver debugging output
On Feb 1 14:23, Lavrentiev, Anton (NIH/NLM/NCBI) [C] via Cygwin-patches wrote:
> > Please use %ls, %S is non-standard.
>
> Sure.
>
> > For instance, write_record appears to handle DNS_TYPE_A,
> > but not DNS_TYPE_.
>
> I can add that, it's not a problem. But indeed, reparsing of Windows packets,
> does miss (as well as some other types, such as URI -- not sure if
> Windows
> has it, though).
>
> > Would you mind to split this into a patchset with patches for different
> > tasks? ATM I'm a bit concerned about the ntoh{sl} calls, given the
> > noticable absence of IPv6 support...
>
> Okay. BTW, I added ntol/s only for output of *nameserver*'s IPv4:port,
> because
> nameservers are IPv4 (even in glibc, AFAIK). The _res structure (same in
> glibc)
> has these addresses as "struct in_addr", meaning they are IPv4.
But nameservers could be v6 addresses nevertheless, and the values are
stored in the _ext.nsaddrs member these days. Our definition of
_res_state does not define all the members of _ext as GLibc defines,
though, and our resolver code doesn't use _ext at all, afaics.
> And so there's
> no risk of running into any troubles, but reading the IP addresses in debug
> output
> is much easier if they are in native order (and same goes for ports, even
> more).
Except, the value has no meaning for ipv6.
Corinna
Re: [PATCH 0/4] getdtablesize, OPEN_MAX, etc.
On Jan 29 14:24, Ken Brown via Cygwin-patches wrote: > This patchset is an extension of the patch submitted here: > > https://cygwin.com/pipermail/cygwin-patches/2021q1/011060.html > > That patch is included as the first patch in this set. The change to > OPEN_MAX still needs testing to see if it has too much impact on the > performance of tcsh. > > I've make a first attempt to implement the suggestion of adding a new > header. At this writing I'm not completely sure > that I fully understand the purpose of that. My choice of which > macros to define in it might need to be changed. > > Ken Brown (4): > Cygwin: getdtablesize: always return OPEN_MAX_MAX > Cygwin: sysconf, getrlimit: don't call getdtablesize > Cygwin: remove the OPEN_MAX_MAX macro > Cygwin: include/cygwin/limits.h: new header > > winsup/cygwin/dtable.cc | 8 +-- > winsup/cygwin/dtable.h| 2 - > winsup/cygwin/fcntl.cc| 2 +- > winsup/cygwin/include/cygwin/limits.h | 65 > winsup/cygwin/include/limits.h| 85 +++ > winsup/cygwin/resource.cc | 5 +- > winsup/cygwin/syscalls.cc | 8 +-- > winsup/cygwin/sysconf.cc | 11 +--- > 8 files changed, 111 insertions(+), 75 deletions(-) > create mode 100644 winsup/cygwin/include/cygwin/limits.h > > -- > 2.30.0 Looks great, please push. Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Feb 1 10:50, Corinna Vinschen via Cygwin-patches wrote: > On Jan 28 15:33, Ken Brown via Cygwin-patches wrote: > > On 1/28/2021 11:07 AM, Corinna Vinschen via Cygwin-patches wrote: > > > One problem is that there are some applications in the wild which run > > > loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open > > > descriptors. tcsh is one of them. It may slow done tcsh quite a bit > > > if the loop runs to 3200 now every time. > > > > I don't use tcsh. Is it easy to test this? > > I just checked the source. In the olden days, before the invention of > close-on-exec, tcsh closed all descriptors > 2 up to OPEN_MAX prior to > starting any executable. > > With close-on-exec this happens only at startup and after an error > occured. > > So testing should be easy: The tcsh startup may be noticably slower. I checked this right now and I don't see a noticable, i. .e, any, difference. Corinna
Re: [PATCH] CYGWIN: Fix resolver debugging output
Hi Anton,
On Jan 29 14:29, Anton Lavrentiev via Cygwin-patches wrote:
> - Use %S (instead of %s) when a wide-character output is due;
Please use %ls, %S is non-standard.
> - Use native byte order for host and add port when doing I/O with DNS server;
This puzzeled me a bit so I took another look into the original code.
Do I see this right that we have only limited IPv6 support in the
resolver code? For instance, write_record appears to handle DNS_TYPE_A,
but not DNS_TYPE_.
CCing Pierre, hopefully he's still around. Pierre, can you please
have a look and suggest how to go forward here?
> - Use forward way for resolv.conf's "options" processing, so listing "debug"
> as a
> first option, will show all following option(s) as they are read;
> - Re-evaluate debug output flag after each "options" processing as it may
> chance.
Would you mind to split this into a patchset with patches for different
tasks? ATM I'm a bit concerned about the ntoh{sl} calls, given the
noticable absence of IPv6 support...
Thanks,
Corinna
Re: [PATCH] Cygwin: exceptions.cc: Suspend all threads in sig_handle_tty_stop().
On Jan 29 12:46, Takashi Yano via Cygwin-patches wrote:
> - Currently, thread created by pthread_create() is not suspended by
> the signal SIGTSTP. For example, even if a process with a thread
> is suspended by Ctrl-Z, the thread continues running. This patch
> fixes the issue.
> ---
> winsup/cygwin/exceptions.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
> index c98b92d30..3a6823325 100644
> --- a/winsup/cygwin/exceptions.cc
> +++ b/winsup/cygwin/exceptions.cc
> @@ -902,7 +902,9 @@ sig_handle_tty_stop (int sig, siginfo_t *, void *)
>thread. */
>/* Use special cygwait parameter to handle SIGCONT. _main_tls.sig will
>be cleared under lock when SIGCONT is detected. */
> + pthread::suspend_all_except_self ();
>DWORD res = cygwait (NULL, cw_infinite, cw_sig_cont);
> + pthread::resume_all ();
>switch (res)
> {
> case WAIT_SIGNALED:
> --
> 2.30.0
Pushed.
Thanks,
Corinna
Re: [PATCH] Cygwin: console: Align the behaviour against signal with pty.
On Jan 29 12:45, Takashi Yano via Cygwin-patches wrote: > - Currently, read() returns -1 with EINTR if the process is suspended > by Ctrl-Z and resumed by fg command, while pty continues to read. > For example, xxd command stops with error "Interrupted system call" > after Ctrl-Z and fg. This patch aligns the behaviour with pty (and > Linux). > --- > winsup/cygwin/fhandler_console.cc | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/winsup/cygwin/fhandler_console.cc > b/winsup/cygwin/fhandler_console.cc > index 0b404411e..3c0783575 100644 > --- a/winsup/cygwin/fhandler_console.cc > +++ b/winsup/cygwin/fhandler_console.cc > @@ -587,7 +587,8 @@ wait_retry: > break; > case input_signalled: /* signalled */ > release_input_mutex (); > - goto sig_exit; > + /* The signal will be handled by cygwait() above. */ > + continue; > case input_winch: > release_input_mutex (); > continue; > -- > 2.30.0 Pushed. Thanks, Corinna
Re: [PATCH v3 0/2] Make terminal read() thread-safe.
On Jan 28 23:11, Takashi Yano via Cygwin-patches wrote: > Currently read() for console and pty slave are somehow > not thread-safe. These patches fix the issue. > > Takashi Yano (2): > Cygwin: console: Make read() thread-safe. > Cygwin: pty: Make slave read() thread-safe. > > winsup/cygwin/fhandler.h | 10 ++ > winsup/cygwin/fhandler_console.cc | 11 ++- > winsup/cygwin/fhandler_termios.cc | 2 ++ > winsup/cygwin/fhandler_tty.cc | 6 ++ > 4 files changed, 24 insertions(+), 5 deletions(-) > > -- > 2.30.0 Pushed. Thanks, Corinna
Re: [PATCH 0/1] Recognizing native Windows AF_UNIX sockets
On Jan 30 11:34, Ken Brown via Cygwin-patches wrote: > This patch attempts to fix the problem reported here: > > https://cygwin.com/pipermail/cygwin/2020-September/246362.html > > See also the followup here: > > https://cygwin.com/pipermail/cygwin/2021-January/247666.html > > The problem, briefly, is that on certain recent versions of Windows > 10, including 2004 but not 1909, native Windows AF_UNIX sockets are > represented by reparse points that Cygwin doesn't recognize. As a > result, tools like 'ls' and 'rm' don't work. LGTM, please push. Thanks, Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 28 15:33, Ken Brown via Cygwin-patches wrote: > On 1/28/2021 11:07 AM, Corinna Vinschen via Cygwin-patches wrote: > > One problem is that there are some applications in the wild which run > > loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open > > descriptors. tcsh is one of them. It may slow done tcsh quite a bit > > if the loop runs to 3200 now every time. > > I don't use tcsh. Is it easy to test this? I just checked the source. In the olden days, before the invention of close-on-exec, tcsh closed all descriptors > 2 up to OPEN_MAX prior to starting any executable. With close-on-exec this happens only at startup and after an error occured. So testing should be easy: The tcsh startup may be noticably slower. Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 29 14:23, Ken Brown via Cygwin-patches wrote: > On 1/28/2021 5:28 PM, Ken Brown via Cygwin-patches wrote: > > > ...ideally by adding a file include/cygwin/limits.h included by > > > include/limits.h, which defines __OPEN_MAX et al, as required. > > > > I'm not completely sure I follow. Do you mean include/cygwin/limits.h > > should contain > > > > #define __OPEN_MAX 3200 > > > > and include/limits.h should contain > > > > #define OPEN_MAX __OPEN_MAX ? > > > > For the sake of my education, could you explain the reason for this? > > Trying to answer my own question, I guess the idea is to hide implementation > details from viewers of limits.h. Is that right? Yes, that was the idea, kind of like a poor mans include/bits dir on Linux... > I took a stab at this and > am about to send a patchset. I'm not sure whether I made a reasonable > choice of "et al" in "__OPEN_MAX et al". Sorry, I didn't mean to imply you will have to do that right away. It was just a thought to move over more values in later patches. Corinna
Re: fhandler_serial.cc: MARK and SPACE parity for serial port
On Jan 29 23:06, Marek Smetana via Cygwin-patches wrote: > Hi, > > I have modified the patch as recommended and am sending it as an attachment. Great, but can you please attach the entire output of `git format-patch -1' including the commit message? Thanks, Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 28 17:07, Corinna Vinschen via Cygwin-patches wrote:
> On Jan 28 08:42, Ken Brown via Cygwin-patches wrote:
> > On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote:
> > > On Jan 27 21:51, Ken Brown via Cygwin-patches wrote:
> > > > According to the Linux man page for getdtablesize(3), the latter is
> > > > supposed to return "the maximum number of files a process can have
> > > > open, one more than the largest possible value for a file descriptor."
> > > > The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
> > > > now return that.
> > > >
> > > > Previously getdtablesize returned the current size of cygheap->fdtab,
> > > > Cygwin's internal file descriptor table. But this is a dynamically
> > > > growing table, and its current size does not reflect an actual limit
> > > > on the number of open files.
> > > >
> > > > With this change, gnulib now reports that getdtablesize and
> > > > fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
> > > > corresponding gnulib modules will no longer use gnulib replacements on
> > > > Cygwin.
> > > > ---
> > > > winsup/cygwin/syscalls.cc | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> > > > index 5da05b18a..1f16d54b9 100644
> > > > --- a/winsup/cygwin/syscalls.cc
> > > > +++ b/winsup/cygwin/syscalls.cc
> > > > @@ -2887,7 +2887,7 @@ setdtablesize (int size)
> > > > extern "C" int
> > > > getdtablesize ()
> > > > {
> > > > - return cygheap->fdtab.size;
> > > > + return OPEN_MAX_MAX;
> > > > }
> > >
> > > getdtablesize is used internally, too. After this change, the values
> > > returned by sysconf and getrlimit should be revisited as well.
> >
> > They will now return OPEN_MAX_MAX, as I think they should. The only
> > question in my mind is whether to simplify the code by removing the calls to
> > getdtablesize, something like this (untested):
>
> But then again, what happens with OPEN_MAX in limits.h? Linux removed
> it entirely. Given we have such a limit and it's not flexible as on
> Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define
> OPEN_MAX as 3200?
...ideally by adding a file include/cygwin/limits.h included by
include/limits.h, which defines __OPEN_MAX et al, as required.
Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 28 08:42, Ken Brown via Cygwin-patches wrote:
> On 1/28/2021 5:20 AM, Corinna Vinschen via Cygwin-patches wrote:
> > On Jan 27 21:51, Ken Brown via Cygwin-patches wrote:
> > > According to the Linux man page for getdtablesize(3), the latter is
> > > supposed to return "the maximum number of files a process can have
> > > open, one more than the largest possible value for a file descriptor."
> > > The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
> > > now return that.
> > >
> > > Previously getdtablesize returned the current size of cygheap->fdtab,
> > > Cygwin's internal file descriptor table. But this is a dynamically
> > > growing table, and its current size does not reflect an actual limit
> > > on the number of open files.
> > >
> > > With this change, gnulib now reports that getdtablesize and
> > > fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
> > > corresponding gnulib modules will no longer use gnulib replacements on
> > > Cygwin.
> > > ---
> > > winsup/cygwin/syscalls.cc | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> > > index 5da05b18a..1f16d54b9 100644
> > > --- a/winsup/cygwin/syscalls.cc
> > > +++ b/winsup/cygwin/syscalls.cc
> > > @@ -2887,7 +2887,7 @@ setdtablesize (int size)
> > > extern "C" int
> > > getdtablesize ()
> > > {
> > > - return cygheap->fdtab.size;
> > > + return OPEN_MAX_MAX;
> > > }
> >
> > getdtablesize is used internally, too. After this change, the values
> > returned by sysconf and getrlimit should be revisited as well.
>
> They will now return OPEN_MAX_MAX, as I think they should. The only
> question in my mind is whether to simplify the code by removing the calls to
> getdtablesize, something like this (untested):
But then again, what happens with OPEN_MAX in limits.h? Linux removed
it entirely. Given we have such a limit and it's not flexible as on
Linux, should we go ahead, drop OPEN_MAX_MAX entirely and define
OPEN_MAX as 3200?
One problem is that there are some applications in the wild which run
loops up to either sysconf(_SC_OPEN_MAX) or OPEN_MAX to handle open
descriptors. tcsh is one of them. It may slow done tcsh quite a bit
if the loop runs to 3200 now every time.
Corinna
Re: [PATCH v7 0/4] Improve pseudo console support.
On Jan 28 12:26, Takashi Yano via Cygwin-patches wrote: > The new implementation of pseudo console support by commit bb428520 > provides the important advantages, while there also has been several > disadvantages compared to the previous implementation. > > These patches overturn some of them. > > The disadvantage: > 1) The cygwin program which calls console API directly does not work. > is supposed to be able to be overcome as well, however, I am not sure > it is worth enough. This will need a lot of hooks for console APIs. > --> Respecting Corinna's opinion, we decided not to implement this. > > v3: Fix typeahead input issue in GDB. Several other bugs have also > been fixed. > v4: Change the conditions for calling transfer_input() slightly in > reset_switch_to_pcon() to avoid calling it if uncecessary or > with no effect. > v5: Small bug fix in v4. > v6: Yet another bug fix. > Add missing CloseHandle(). > Take into account when the master is running as a service (such > as ssh session). > v7: Specify FILE_FLAG_OVERLAPPED for to_slave pipe to prevent > PeekNamedPipe() from blocking in transfer_input(). > Simplify the code determining if the slave is reading. > > Takashi Yano (4): > Cygwin: pty: Inherit typeahead data between two input pipes. > Cygwin: pty: Keep code page between non-cygwin apps. > Cygwin: pty: Make apps using console APIs be able to debug with gdb. > Cygwin: pty: Allow multiple apps to enable pseudo console > simultaneously. > > winsup/cygwin/fhandler.h | 22 +- > winsup/cygwin/fhandler_tty.cc | 1123 +++-- > winsup/cygwin/select.cc |7 +- > winsup/cygwin/spawn.cc| 106 +++- > winsup/cygwin/tty.cc | 13 +- > winsup/cygwin/tty.h | 21 +- > 6 files changed, 1059 insertions(+), 233 deletions(-) > > -- > 2.30.0 Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: getdtablesize: always return OPEN_MAX_MAX
On Jan 27 21:51, Ken Brown via Cygwin-patches wrote:
> According to the Linux man page for getdtablesize(3), the latter is
> supposed to return "the maximum number of files a process can have
> open, one more than the largest possible value for a file descriptor."
> The constant OPEN_MAX_MAX is the only limit enforced by Cygwin, so we
> now return that.
>
> Previously getdtablesize returned the current size of cygheap->fdtab,
> Cygwin's internal file descriptor table. But this is a dynamically
> growing table, and its current size does not reflect an actual limit
> on the number of open files.
>
> With this change, gnulib now reports that getdtablesize and
> fcntl(F_DUPFD) work on Cygwin. Packages like GNU tar that use the
> corresponding gnulib modules will no longer use gnulib replacements on
> Cygwin.
> ---
> winsup/cygwin/syscalls.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 5da05b18a..1f16d54b9 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -2887,7 +2887,7 @@ setdtablesize (int size)
> extern "C" int
> getdtablesize ()
> {
> - return cygheap->fdtab.size;
> + return OPEN_MAX_MAX;
> }
getdtablesize is used internally, too. After this change, the values
returned by sysconf and getrlimit should be revisited as well.
Thanks,
Corinna
Re: fhandler_serial.cc: MARK and SPACE parity for serial port
Oh, btw... On Jan 28 11:08, Corinna Vinschen via Cygwin-patches wrote: > Hi Marek, > > thanks for the patch. [...] > > index 17e8d83a3..933851c21 100644 > > --- a/winsup/cygwin/include/sys/termios.h > > +++ b/winsup/cygwin/include/sys/termios.h > > @@ -185,6 +185,7 @@ POSIX commands */ > > #define PARODD 0x00200 > > #define HUPCL 0x00400 > > #define CLOCAL 0x00800 > > +#define CMSPAR 0x4000 /* Mark or space (stick) parity. */ Why did you choose such a big value here? Wouldn't it be nicer just to follow up with #define CMSPAR 0x1 or am I missing something here? Also, on second thought I think CMSPAR should follow CRTSCTS, a few lines below, because of its numerical value higher than CRTSCTS. Thanks, Corinna
Re: fhandler_serial.cc: MARK and SPACE parity for serial port
Hi Marek, thanks for the patch. This is a patch adding functionality so it's not trivial. Would you mind to express your willingness to put this patch as well as further patches under 2-clause BSD license per the "Before you get started" section of https://cygwin.com/contrib.html? A few minor problems with this patch. First of all, your MUA apparently broke the inline patch. There are line breaks in the patch, unexpected by git am, and the spacing in the termios.h hunk is all wrong, see below. If you can't change that in your MUA, please attach your patches as plain text attachements, that usually helps. On Jan 27 21:30, Marek Smetana via Cygwin-patches wrote: > Hi, > > This patch add MARK and SPACE parity support to serial port > > --- > winsup/cygwin/fhandler_serial.cc| 9 - > winsup/cygwin/include/sys/termios.h | 1 + > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/winsup/cygwin/fhandler_serial.cc > b/winsup/cygwin/fhandler_serial.cc Wrong line break > index fd5b45899..23d69eca5 100644 > --- a/winsup/cygwin/fhandler_serial.cc > +++ b/winsup/cygwin/fhandler_serial.cc > @@ -727,7 +727,10 @@ fhandler_serial::tcsetattr (int action, const struct > termios *t) Wrong line break >/* -- Set parity -- */ > >if (t->c_cflag & PARENB) > -state.Parity = (t->c_cflag & PARODD) ? ODDPARITY : EVENPARITY; > +if(t->c_cflag & CMSPAR) > + state.Parity = (t->c_cflag & PARODD) ? MARKPARITY : SPACEPARITY; > +else > + state.Parity = (t->c_cflag & PARODD) ? ODDPARITY : EVENPARITY; Please put the nested if/else into curly braces so the potential for later changes breaking the if/else chain is reduced. >else > state.Parity = NOPARITY; > > @@ -1068,6 +1071,10 @@ fhandler_serial::tcgetattr (struct termios *t) > t->c_cflag |= (PARENB | PARODD); >if (state.Parity == EVENPARITY) > t->c_cflag |= PARENB; > + if (state.Parity == MARKPARITY) > +t->c_cflag |= (PARENB | PARODD | CMSPAR); > + if (state.Parity == SPACEPARITY) > +t->c_cflag |= (PARENB | CMSPAR); > >/* -- Parity errors -- */ > > diff --git a/winsup/cygwin/include/sys/termios.h > b/winsup/cygwin/include/sys/termios.h Wrong line break > index 17e8d83a3..933851c21 100644 > --- a/winsup/cygwin/include/sys/termios.h > +++ b/winsup/cygwin/include/sys/termios.h > @@ -185,6 +185,7 @@ POSIX commands */ > #define PARODD 0x00200 > #define HUPCL 0x00400 > #define CLOCAL 0x00800 > +#define CMSPAR 0x4000 /* Mark or space (stick) parity. */ Spacing here is completely off, probably due to your MUA. Please note that every definition is followed by a TAB and a SPACE for historical reasons. Ideally you do the same for the new CMSPAR definition. > > /* Extended baud rates above 37K. */ > #define CBAUDEX 0x0100f > > --- Other than these minor formatting issues, your patch looks good, so I'm looking forward to the fixed version. Thanks, Corinna
Re: [PATCH v2] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
On Jan 27 13:53, Ken Brown via Cygwin-patches wrote:
> Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
> non-symlinks. Previously it always failed, as it does on Linux. But
> POSIX permits it to succeed on non-symlinks even if it fails on
> symlinks.
>
> The reason for following POSIX rather than Linux is to make gnulib
> report that fchmodat works on Cygwin. This improves the efficiency of
> packages like GNU tar that use gnulib's fchmodat module. Previously
> such packages would use a gnulib replacement for fchmodat on Cygwin.
> ---
> winsup/cygwin/syscalls.cc | 20 +++-
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 4cc8d07f5..5da05b18a 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -4787,17 +4787,27 @@ fchmodat (int dirfd, const char *pathname, mode_t
> mode, int flags)
>tmp_pathbuf tp;
>__try
> {
> - if (flags)
> + if (flags & ~AT_SYMLINK_NOFOLLOW)
> {
> - /* BSD has lchmod, but Linux does not. POSIX says
> - AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks; but Linux
> - blindly fails even for non-symlinks. */
> - set_errno ((flags & ~AT_SYMLINK_NOFOLLOW) ? EINVAL : EOPNOTSUPP);
> + set_errno (EINVAL);
> __leave;
> }
>char *path = tp.c_get ();
>if (gen_full_path_at (path, dirfd, pathname))
> __leave;
> + if (flags)
For clarity, and on the off-chance that new flags are added to fchmodat,
it might be better to check for (flags & AT_SYMLINK_NOFOLLOW) here
explicitely. Your choice.
> + {
> + /* BSD has lchmod, but Linux does not. POSIX says
> + AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks.
> + Linux blindly fails even for non-symlinks, but we allow
> + it to succeed. */
> + path_conv pc (path, PC_SYM_NOFOLLOW, stat_suffixes);
> + if (pc.issymlink ())
> + {
> + set_errno (EOPNOTSUPP);
> + __leave;
> + }
> + }
>return chmod (path, mode);
> }
>__except (EFAULT) {}
> --
> 2.30.0
Looks good.
Thanks,
Corinna
Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
On Jan 27 08:22, Ken Brown via Cygwin-patches wrote: > On 1/27/2021 7:40 AM, Corinna Vinschen via Cygwin-patches wrote: > > On Jan 26 16:30, Ken Brown via Cygwin-patches wrote: > > > Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on > > > non-symlinks. Previously it always failed, as it does on Linux. But > > > POSIX permits it to succeed on non-symlinks even if it fails on > > > symlinks. > > > > > > The reason for following POSIX rather than Linux is to make gnulib > > > report that fchmodat works on Cygwin. This improves the efficiency of > > > packages like GNU tar that use gnulib's fchmodat module. Previously > > > such packages would use a gnulib replacement for fchmodat on Cygwin. > > > > Wait, what? So if Cygwin behaves like Linux, gnulib treats fchmodat > > as non-working? So what does gnulib do on a Linux system? Does it > > use its own fchmodat there, too? > > Apparently so. Here's a comment from gnulib's test program for fchmodat: > > /* Test whether fchmodat+AT_SYMLINK_NOFOLLOW works on > non-symlinks. > This test fails on GNU/Linux with glibc 2.31 (but not on > GNU/kFreeBSD nor GNU/Hurd) and Cygwin 2.9. */ > > I agree that it's strange. ¯\_(ツ)_/¯ Corinna
Re: [PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
On Jan 26 16:30, Ken Brown via Cygwin-patches wrote: > Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on > non-symlinks. Previously it always failed, as it does on Linux. But > POSIX permits it to succeed on non-symlinks even if it fails on > symlinks. > > The reason for following POSIX rather than Linux is to make gnulib > report that fchmodat works on Cygwin. This improves the efficiency of > packages like GNU tar that use gnulib's fchmodat module. Previously > such packages would use a gnulib replacement for fchmodat on Cygwin. Wait, what? So if Cygwin behaves like Linux, gnulib treats fchmodat as non-working? So what does gnulib do on a Linux system? Does it use its own fchmodat there, too? Puzzled, Corinna
Re: [PATCH] Cygwin: chown: make sure ctime gets updated when necessary
On Jan 25 14:16, Ken Brown via Cygwin-patches wrote:
> On 1/25/2021 1:57 PM, Corinna Vinschen via Cygwin-patches wrote:
> > On Jan 25 12:24, Ken Brown via Cygwin-patches wrote:
> > > Following POSIX, ensure that ctime is updated if chown succeeds,
> > > unless the new owner is specified as (uid_t)-1 and the new group is
> > > specified as (gid_t)-1. Previously, ctime was unchanged whenever the
> > > owner and group were both unchanged.
> > >
> > > Aside from POSIX compliance, this fix makes gnulib report that chown
> > > works on Cygwin. This improves the efficiency of packages like GNU
> > > tar that use gnulib's chown module. Previously such packages would
> > > use a gnulib replacement for chown on Cygwin.
> > > ---
> > > winsup/cygwin/fhandler_disk_file.cc | 10 +-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/winsup/cygwin/fhandler_disk_file.cc
> > > b/winsup/cygwin/fhandler_disk_file.cc
> > > index 07f9c513a..72d259579 100644
> > > --- a/winsup/cygwin/fhandler_disk_file.cc
> > > +++ b/winsup/cygwin/fhandler_disk_file.cc
> > > @@ -863,6 +863,7 @@ fhandler_disk_file::fchown (uid_t uid, gid_t gid)
> > > tmp_pathbuf tp;
> > > aclent_t *aclp;
> > > int nentries;
> > > + bool noop = true;
> > > if (!pc.has_acls ())
> > > {
> > > @@ -887,11 +888,18 @@ fhandler_disk_file::fchown (uid_t uid, gid_t gid)
> > > aclp, MAX_ACL_ENTRIES)) < 0)
> > > goto out;
> > > + /* According to POSIX, chown can be a no-op if uid is (uid_t)-1 and
> > > + gid is (gid_t)-1. Otherwise, even if uid and gid are unchanged,
> > > + we must ensure that ctime is updated. */
> > > if (uid == ILLEGAL_UID)
> > > uid = old_uid;
> > > + else
> > > +noop = false;
> > > if (gid == ILLEGAL_GID)
> > > gid = old_gid;
> > > - if (uid == old_uid && gid == old_gid)
> >
> > Basically ok, but why not just
> >
> > if (uid == ILLEGAL_UID && gid == ILLEGAL_GID)
> >
> > instead of the noop var?
>
> I went back and forth on that. Following your suggestion, the code looks
> like this:
>
> if (uid == ILLEGAL_UID && gid == ILLEGAL_GID)
> {
> ret = 0;
> goto out;
> }
> if (uid == ILLEGAL_UID)
> uid = old_uid;
> if (gid == ILLEGAL_GID)
> gid = old_gid;
>
> I was trying to avoid checking uid == ILLEGAL_UID and gid == ILLEGAL_GID
> twice. But on second thought, it's probably silly to worry about that. The
> code is cleaner without the noop variable.
Both is ok with me, whatever you think more spiffy here.
Thanks,
Corinna
Re: [PATCH v2 7/8] dir.cc: Try unlink_nt first
On Jan 20 17:10, Ben Wijen wrote:
> Speedup deletion of directories.
> ---
> winsup/cygwin/dir.cc | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc
> index 7762557d6..470f83aee 100644
> --- a/winsup/cygwin/dir.cc
> +++ b/winsup/cygwin/dir.cc
> @@ -22,6 +22,8 @@ details. */
> #include "cygtls.h"
> #include "tls_pbuf.h"
>
> +extern NTSTATUS unlink_nt (const char *ourname, ULONG eflags);
> +
> extern "C" int
> dirfd (DIR *dir)
> {
> @@ -398,6 +400,14 @@ rmdir (const char *dir)
> if (msdos && p == dir + 1 && isdrive (dir))
> p[1] = '\\';
> }
> + if (has_dot_last_component (dir, false)) {
> +set_errno (EINVAL);
> +__leave;
> + }
> + if (NT_SUCCESS (unlink_nt (dir, FILE_DIRECTORY_FILE))) {
> +res = 0;
> +__leave;
> + }
So what about /dev, /proc, etc?
>if (!(fh = build_fh_name (dir, PC_SYM_NOFOLLOW)))
> __leave; /* errno already set */;
>
> @@ -408,8 +418,6 @@ rmdir (const char *dir)
> }
>else if (!fh->exists ())
> set_errno (ENOENT);
> - else if (has_dot_last_component (dir, false))
> - set_errno (EINVAL);
>else if (!fh->rmdir ())
> res = 0;
>delete fh;
Corinna
Re: [PATCH v2 6/8] syscalls.cc: Expose shallow-pathconv unlink_nt
On Jan 20 17:10, Ben Wijen wrote:
> Not having to query file information improves unlink speed.
> ---
> winsup/cygwin/syscalls.cc | 78 ++-
> 1 file changed, 52 insertions(+), 26 deletions(-)
>
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index ab0c4c2d6..b5ab6ac5e 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -1272,6 +1272,28 @@ _unlink_ntpc_ (path_conv& pc, bool shareable)
>return status;
> }
>
> +NTSTATUS
> +unlink_nt (const char *ourname, ULONG eflags)
> +{
> + uint32_t opt = PC_SYM_NOFOLLOW | PC_SKIP_SYM_CHECK | PC_SKIP_FS_CHECK;
> + if (!(eflags & FILE_NON_DIRECTORY_FILE))
> +opt &= ~PC_SKIP_FS_CHECK;
> +
> + path_conv pc (ourname, opt, NULL);
> + if (pc.error || pc.isspecial ())
> +return STATUS_CANNOT_DELETE;
> +
> + OBJECT_ATTRIBUTES attr;
> + PUNICODE_STRING ntpath = pc.get_nt_native_path ();
> + InitializeObjectAttributes(, ntpath, 0, NULL, NULL);
> + NTSTATUS status = _unlink_nt (, eflags);
Sorry again, but I don't see the advantage of not using the intelligence
already collected in path_conv by neglecting to call the unlink
variation not using them. It's also unclear to me, why the new code
doesn't try_to_bin right away, rather than stomping ahead and falling
back to the current code for each such file. In an rm -rf on a file
hirarchy used by other users, you could end up with a much slower
operation.
Wouldn't it make more sense to streamline the existing _unlink_nt? I
don't claim it's the most streamlined way to delete files, but at least
it has documented workarounds for problems encountered on the way,
already.
Corinna
Re: [PATCH v2 4/8] syscalls.cc: Implement non-path_conv dependent _unlink_nt
Hi Ben,
ok, this is strong stuff, and apart from a couple of formatting issues,
we should really discuss if a couple of things are feasible at all.
On Jan 20 17:10, Ben Wijen wrote:
> Implement _unlink_nt: wich does not depend on patch_conv
> ---
> winsup/cygwin/fhandler_disk_file.cc | 4 +-
> winsup/cygwin/forkable.cc | 4 +-
> winsup/cygwin/syscalls.cc | 211 ++--
> 3 files changed, 200 insertions(+), 19 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_disk_file.cc
> b/winsup/cygwin/fhandler_disk_file.cc
> index 07f9c513a..fe04f832b 100644
> --- a/winsup/cygwin/fhandler_disk_file.cc
> +++ b/winsup/cygwin/fhandler_disk_file.cc
> @@ -1837,7 +1837,7 @@ fhandler_disk_file::mkdir (mode_t mode)
> int
> fhandler_disk_file::rmdir ()
> {
> - extern NTSTATUS unlink_nt (path_conv );
> + extern NTSTATUS unlink_ntpc (path_conv );
First of all, the new function should better get a new name. The _nt
postfix is pretty much historical anyway to differentiate between the
function using Win32 API and the function using NT API. This is kind
of moot these days sine we're using the NT API almost exclusively for
file access anyway.
So stick to unlink_nt/_unlink_nt for the existing functions, and name
the new function accorind to it's doings, like, say, unlink_path or
whatever.
> @@ -695,7 +695,157 @@ _unlink_nt_post_dir_check (NTSTATUS status,
> POBJECT_ATTRIBUTES attr, const path_
> }
>
> static NTSTATUS
> -_unlink_nt (path_conv , bool shareable)
> +_unlink_nt (POBJECT_ATTRIBUTES attr, ULONG eflags)
> +{
> + static bool has_posix_unlink_semantics =
> + wincap.has_posix_unlink_semantics ();
> + static bool has_posix_unlink_semantics_with_ignore_readonly =
> + wincap.has_posix_unlink_semantics_with_ignore_readonly ();
Did you mean `const' rather than `static', by any chance? Either way, I
don't think these local vars are required, given that the wincap
accessors are already marked as const. The compiler should know how to
opimize this sufficiently.
> +
> + HANDLE fh;
> + ACCESS_MASK access = DELETE;
> + IO_STATUS_BLOCK io;
> + ULONG flags = FILE_OPEN_REPARSE_POINT | FILE_OPEN_FOR_BACKUP_INTENT
> + | FILE_DELETE_ON_CLOSE | eflags;
This looks like a dangerous assumption. So far we don't open unknown
reparse points as reparse points deliberately. No one knows what a
unknown reparse point is good for or supposed to do, so we don't even
know if we are allowed to handle it analogue to a symlink.
Consequentially we open unknown reparse points just as normal files, so
that the reparse point's automatisms may kick in. By omitting this
step, we're moving on thin ice.
> + NTSTATUS fstatus, istatus = STATUS_SUCCESS;
> +
> + syscall_printf("Trying to delete %S, isdir = %d", attr->ObjectName,
> + eflags == FILE_DIRECTORY_FILE);
> +
> + //FILE_DELETE_ON_CLOSE icw FILE_DIRECTORY_FILE only works when directory
> is empty
> + //-> We must assume directory not empty, therefore only do this for
> regular files.
Please use C-style /* ... */ comments in the first place, especially
on multi-line comments.
Also, please keep the line length below 80 chars where possible.
> + if (eflags & FILE_NON_DIRECTORY_FILE)
> +{
> + //Step 1
> + //If the file is not 'in use' and not 'readonly', this should just
> work.
> + fstatus = NtOpenFile (, access, attr, , FILE_SHARE_DELETE,
> flags);
> + debug_printf ("NtOpenFile %S: %y", attr->ObjectName, fstatus);
> +}
> +
> + if (!(eflags & FILE_NON_DIRECTORY_FILE)// Workaround for the
> non-empty-dir issue
> + || fstatus == STATUS_SHARING_VIOLATION // The file is 'in use'
> + || fstatus == STATUS_CANNOT_DELETE)// The file is 'readonly'
I'd drop these comments, the status codes are somewhat self-explaining.
> +{
> + //Step 2
> + //Reopen with all sharing flags, will set delete flag ourselves.
> + access |= FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES;
> + flags &= ~FILE_DELETE_ON_CLOSE;
> + fstatus = NtOpenFile (, access, attr, , FILE_SHARE_VALID_FLAGS,
> flags);
> + debug_printf ("NtOpenFile %S: %y", attr->ObjectName, fstatus);
> +
> + if (NT_SUCCESS (fstatus))
> +{
> + if (has_posix_unlink_semantics_with_ignore_readonly)
> +{
> + //Step 3
> + //Remove the file with POSIX unlink semantics, ignore readonly
> flags.
No check for NTFS? Posix semantics are not supported on any other FS.
No check for remote? Just because you support POSIX semantics on
*this* machine, doesn't mean the remote machine supports it at all...
> + FILE_DISPOSITION_INFORMATION_EX fdie =
> +{ FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS
> +| FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE };
> + istatus = NtSetInformationFile (fh, , , sizeof fdie,
> +
Re: [PATCH v3 2/8] syscalls.cc: Deduplicate remove
On Jan 22 16:47, Ben Wijen wrote: > The remove code is already in the _remove_r function. > So, just call the _remove_r function. > --- > winsup/cygwin/syscalls.cc | 17 - > 1 file changed, 4 insertions(+), 13 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: chown: make sure ctime gets updated when necessary
On Jan 25 12:24, Ken Brown via Cygwin-patches wrote:
> Following POSIX, ensure that ctime is updated if chown succeeds,
> unless the new owner is specified as (uid_t)-1 and the new group is
> specified as (gid_t)-1. Previously, ctime was unchanged whenever the
> owner and group were both unchanged.
>
> Aside from POSIX compliance, this fix makes gnulib report that chown
> works on Cygwin. This improves the efficiency of packages like GNU
> tar that use gnulib's chown module. Previously such packages would
> use a gnulib replacement for chown on Cygwin.
> ---
> winsup/cygwin/fhandler_disk_file.cc | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/fhandler_disk_file.cc
> b/winsup/cygwin/fhandler_disk_file.cc
> index 07f9c513a..72d259579 100644
> --- a/winsup/cygwin/fhandler_disk_file.cc
> +++ b/winsup/cygwin/fhandler_disk_file.cc
> @@ -863,6 +863,7 @@ fhandler_disk_file::fchown (uid_t uid, gid_t gid)
>tmp_pathbuf tp;
>aclent_t *aclp;
>int nentries;
> + bool noop = true;
>
>if (!pc.has_acls ())
> {
> @@ -887,11 +888,18 @@ fhandler_disk_file::fchown (uid_t uid, gid_t gid)
> aclp, MAX_ACL_ENTRIES)) < 0)
> goto out;
>
> + /* According to POSIX, chown can be a no-op if uid is (uid_t)-1 and
> + gid is (gid_t)-1. Otherwise, even if uid and gid are unchanged,
> + we must ensure that ctime is updated. */
>if (uid == ILLEGAL_UID)
> uid = old_uid;
> + else
> +noop = false;
>if (gid == ILLEGAL_GID)
> gid = old_gid;
> - if (uid == old_uid && gid == old_gid)
Basically ok, but why not just
if (uid == ILLEGAL_UID && gid == ILLEGAL_GID)
instead of the noop var?
Corinna
Re: [PATCH] Cygwin: console: Add missing guard regarding attach_mutex.
On Jan 25 18:18, Takashi Yano via Cygwin-patches wrote: > - The commit a545 did not fix the problem enough. This patch > provides additional guard for the issue. > --- > winsup/cygwin/fhandler_console.cc | 27 +++ > 1 file changed, 27 insertions(+) Pushed. Thanks, Corinna
Re: [PATCH v2 2/8] syscalls.cc: Deduplicate remove
On Jan 20 17:10, Ben Wijen wrote:
> The remove code is already in the _remove_r function.
> So, just call the _remove_r function.
> ---
> winsup/cygwin/syscalls.cc | 17 -
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 2e50ad7d5..54b065733 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -1133,24 +1133,15 @@ _remove_r (struct _reent *, const char *ourname)
>return -1;
> }
>
> - return win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
> + int res = win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
> + syscall_printf ("%R = remove(%s)", res, ourname);
> + return res;
> }
>
> extern "C" int
> remove (const char *ourname)
> {
> - path_conv win32_name (ourname, PC_SYM_NOFOLLOW);
> -
> - if (win32_name.error)
> -{
> - set_errno (win32_name.error);
> - syscall_printf ("-1 = remove (%s)", ourname);
> - return -1;
> -}
> -
> - int res = win32_name.isdir () ? rmdir (ourname) : unlink (ourname);
> - syscall_printf ("%R = remove(%s)", res, ourname);
> - return res;
> + return _remove_r(_REENT, ourname);
^^^
space
Corinna
Re: [PATCH v2 0/4] Improve pseudo console support.
Hi Takashi, On Jan 22 05:58, Takashi Yano via Cygwin-patches wrote: > The new implementation of pseudo console support by commit bb428520 > provides the important advantages, while there also has been several > disadvantages compared to the previous implementation. > > These patches overturn some of them. > > The disadvantage: > 1) The cygwin program which calls console API directly does not work. > is supposed to be able to be overcome as well, however, I am not sure > it is worth enough. This will need a lot of hooks for console APIs. > > Takashi Yano (4): > Cygwin: pty: Inherit typeahead data between two input pipes. > Cygwin: pty: Keep code page between non-cygwin apps. > Cygwin: pty: Make apps using console APIs be able to debug with gdb. > Cygwin: pty: Allow multiple apps to enable pseudo console > simultaneously. > > winsup/cygwin/fhandler.h | 15 +- > winsup/cygwin/fhandler_tty.cc | 805 ++ > winsup/cygwin/spawn.cc| 102 +++-- > winsup/cygwin/tty.cc | 11 +- > winsup/cygwin/tty.h | 18 +- > 5 files changed, 730 insertions(+), 221 deletions(-) > > -- > 2.30.0 I found a problem with this patchset. Try this: Start mintty $ touch foo $ attrib +r foo $ gdb /bin/rm $ start foo At this point, starting rm will take a few seconds. While GDB is still working on this, *before* GDB returns to the prompt, type some keys on keyboard, e. g., "1234". Without this patchset, you'll see the keys being echoed in mintty, and as soon as GDB returns to the prompt, the keys are copied to GDBs input buffer and the keys you typed show up after the prompt. This is the expected behaviour. (gdb) 1234 With this patchset, the keys are *not* echoed in mintty, and as soon as the GDB prompt returns, the keys are still not visible. Now continue the execution of rm: (gdb) c /usr/bin/rm: remove write-protected regular file 'foo'? Without this patchset, I get /usr/bin/rm: error closing file [...] [Inferior 1 (process 1224) exited with code 01] (gdb) That's not optimal, apparently. With this patchset: (gdb) c /usr/bin/rm: remove write-protected regular file 'foo'? 1234 so the keys typed while gdb was starting rm have been saved up and then used as input for rm. That's not quite right either, is it? Thanks, Corinna
Re: [PATCH v2 0/4] Improve pseudo console support.
On Jan 22 05:58, Takashi Yano via Cygwin-patches wrote: > The new implementation of pseudo console support by commit bb428520 > provides the important advantages, while there also has been several > disadvantages compared to the previous implementation. > > These patches overturn some of them. > > The disadvantage: > 1) The cygwin program which calls console API directly does not work. > is supposed to be able to be overcome as well, however, I am not sure > it is worth enough. This will need a lot of hooks for console APIs. Definitely not. We should really not cave in to such expectations. Cygwin apps are POSIX apps in the first place and should use the API provided by Cygwin and other Cygwin libs. Yes, there are border cases like the X server or cygrunsrv, but these are limited and should stay limited. Corinna
Re: [PATCH] Cygwin: normalize_posix_path: fix error handling when .. is encountered
On Jan 20 10:40, Ken Brown via Cygwin-patches wrote: > When .. is in the source path and the path prefix exists but is not a > directory, return ENOTDIR instead of ENOENT. This fixes a failing > gnulib test of realpath(3). > > Addresses: https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00214.html > --- > winsup/cygwin/path.cc | 4 +++- > winsup/cygwin/release/3.2.0 | 4 > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc > index abd3687df..6dc162806 100644 > --- a/winsup/cygwin/path.cc > +++ b/winsup/cygwin/path.cc > @@ -323,8 +323,10 @@ normalize_posix_path (const char *src, char *dst, char > *) > if (!tp.check_usage (4, 3)) > return ELOOP; > path_conv head (dst, PC_SYM_FOLLOW | PC_POSIX); > - if (!head.isdir()) > + if (!head.exists ()) > return ENOENT; > + if (!head.isdir ()) > + return ENOTDIR; > /* At this point, dst is a normalized path. If the >normalized path created by path_conv does not >match the normalized path we're just testing, then > diff --git a/winsup/cygwin/release/3.2.0 b/winsup/cygwin/release/3.2.0 > index c18a848de..43725cec2 100644 > --- a/winsup/cygwin/release/3.2.0 > +++ b/winsup/cygwin/release/3.2.0 > @@ -48,3 +48,7 @@ Bug Fixes > > - Fix a bug in fstatat(2) on 32 bit that could cause it to return garbage. >Addresses: https://cygwin.com/pipermail/cygwin/2021-January/247399.html > + > +- Fix the errno when a path contains .. and the prefix exists but is > + not a directory. > + Addresses: > https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00214.html > -- > 2.30.0 Ok, please push. Thanks, Corinna
Re: [PATCH] Cygwin: ptsname_r: always return an error number on failure
On Jan 21 17:48, Ken Brown via Cygwin-patches wrote:
> On 1/20/2021 1:00 PM, Ken Brown via Cygwin-patches wrote:
> > Following Linux, return ENOTTY on a bad file descriptor and also set
> > errno to ENOTTY.
> >
> > Previously 0 was returned and errno was set to EBADF. Returning 0
> > violates the requirement in
> > https://man7.org/linux/man-pages/man3/ptsname_r.3.html that an error
> > number should be returned on failure. (That man page doesn't specify
> > setting errno.)
> >
> > Addresses:
> > https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00245.html
> > ---
> > winsup/cygwin/release/3.2.0 | 3 +++
> > winsup/cygwin/syscalls.cc | 5 -
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/release/3.2.0 b/winsup/cygwin/release/3.2.0
> > index 43725cec2..f748a9bc8 100644
> > --- a/winsup/cygwin/release/3.2.0
> > +++ b/winsup/cygwin/release/3.2.0
> > @@ -52,3 +52,6 @@ Bug Fixes
> > - Fix the errno when a path contains .. and the prefix exists but is
> > not a directory.
> > Addresses:
> > https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00214.html
> > +
> > +- Fix the return value when ptsname_r(3) is called with a bad file
> > descriptor
> > + Addresses:
> > https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00245.html
> > diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> > index 4742c6653..18d9e3f88 100644
> > --- a/winsup/cygwin/syscalls.cc
> > +++ b/winsup/cygwin/syscalls.cc
> > @@ -3364,7 +3364,10 @@ ptsname_r (int fd, char *buf, size_t buflen)
> > cygheap_fdget cfd (fd);
> > if (cfd < 0)
> > -return 0;
> > +{
> > + set_errno (ENOTTY);
> > + return ENOTTY;
> > +}
> > return cfd->ptsname_r (buf, buflen);
> > }
> >
>
> I'm not really convinced we should blindly follow Linux here, when EBADF
> would seem to make more sense. See
>
> https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00264.html
>
> Corinna, what's your preference?
EBADF actually makes more sense, as Bruno points out.
Please push, whatever you prefer.
Thanks,
Corinna
Re: [PATCH] Cygwin: pty: Reduce buffer size in get_console_process_id().
On Jan 20 19:40, Takashi Yano via Cygwin-patches wrote: > On Wed, 20 Jan 2021 10:50:24 +0100 > Corinna Vinschen wrote: > > On Jan 20 09:57, Takashi Yano via Cygwin-patches wrote: > > > - The buffer used in get_console_process_id(), introduced by commit > > > 72770148, is too large and ERROR_NOT_ENOUGH_MEMORY occurs in Win7. > > > > Huh, funny! Will we ever be happy with just 8192 processes per > > console? :) > > According to my test, when the buffer size is larger than 15683, > this error occurs. Test environment is Win7 x64. Both inside and > outside of WOW64, the maximum allowed size seems to be the same. > > Shall we increase to 15683? :p Ugh, please, no :D 8K processes per console is fine. The machine will probably be out of memory before this is a concern. Corinna
Re: [PATCH] Cygwin: console: Fix "Bad file descriptor" error in script command.
On Jan 20 18:16, Takashi Yano via Cygwin-patches wrote: > - After the commit 72770148, script command exits occasionally with > the error "Bad file descriptor" if it is started in console on Win7 > and non-cygwin process is executed. This patch fixes the issue. > --- > winsup/cygwin/fhandler_console.cc | 10 ++-- > winsup/cygwin/select.cc | 95 ++- > winsup/cygwin/select.h| 7 +++ > 3 files changed, 105 insertions(+), 7 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Reduce buffer size in get_console_process_id().
On Jan 20 09:57, Takashi Yano via Cygwin-patches wrote: > - The buffer used in get_console_process_id(), introduced by commit > 72770148, is too large and ERROR_NOT_ENOUGH_MEMORY occurs in Win7. Huh, funny! Will we ever be happy with just 8192 processes per console? :) > Therefore, the buffer size has been reduced. > --- > winsup/cygwin/fhandler_tty.cc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Pushed. THanks, Corinna
Re: [PATCH] Cygwin: pty: Lessen the side effect of workaround for rlwarp.
On Jan 19 18:27, Takashi Yano via Cygwin-patches wrote:
> - This patch lessens the side effect of the workaround for rlwrap
> introduced by commit 4e16b033.
> ---
> winsup/cygwin/fhandler_tty.cc | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index 473c0c968..c78e996e8 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -1176,11 +1176,19 @@ fhandler_pty_slave::tcgetattr (struct termios *t)
> {
>reset_switch_to_pcon ();
>*t = get_ttyp ()->ti;
> +
>/* Workaround for rlwrap */
> - if (get_ttyp ()->pcon_start)
> -t->c_lflag &= ~(ICANON | ECHO);
> - if (get_ttyp ()->h_pseudo_console)
> -t->c_iflag &= ~ICRNL;
> + cygheap_fdenum cfd (false);
> + while (cfd.next () >= 0)
> +if (cfd->get_major () == DEV_PTYM_MAJOR
> + && cfd->get_minor () == get_minor ())
> + {
> + if (get_ttyp ()->pcon_start)
> + t->c_lflag &= ~(ICANON | ECHO);
> + if (get_ttyp ()->h_pseudo_console)
> + t->c_iflag &= ~ICRNL;
> + break;
> + }
>return 0;
> }
>
> --
> 2.30.0
Pushed.
Thanks,
Corinna
Re: [PATCH] Cygwin: spawn.cc: Fix typo in comment by commit 974e6d76.
On Jan 19 03:45, Takashi Yano via Cygwin-patches wrote:
> ---
> winsup/cygwin/spawn.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index 42044ab53..d03492ee6 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -636,7 +636,7 @@ child_info_spawn::worker (const char *prog_arg, const
> char *const *argv,
> }
>struct fhandler_console::handle_set_t cons_handle_set = { 0, };
>if (cons_native)
> - /* Console handles will be closed by close_all_handle(),
> + /* Console handles will be closed by close_all_files(),
> therefore, duplicate them here */
> cons_native->get_duplicated_handle_set (_handle_set);
>
> --
> 2.30.0
Pushed.
Thanks,
Corinna
Re: [PATCH 11/11] dir.cc: Try unlink_nt first
On Jan 18 18:07, Ben wrote: > > > On 18-01-2021 13:13, Corinna Vinschen via Cygwin-patches wrote: > > > > Your code is skipping the safety checks and the has_dot_last_component() > > check. The latter implements a check required by POSIX. Skipping > > it introduces an incompatibility, see man 2 rmdir. > > > > Yes, I missed has_dot_last_component completely. > > As for the other checks: > dir.cc: 404: fh->error (): > * Done in unlink_nt > dir.cc: 409: fh->exists (): > * Done in _unlink_nt through NtOpenFile, which will return either > STATUS_OBJECT_NAME_NOT_FOUND or STATUS_OBJECT_PATH_NOT_FOUND, > both of which resolve to ENOENT > dir.cc: 413: isdev_dev (fh->dev ()): > * Done in unlink_nt > fhandler_siak_file.cc: 1842: if (!pc.isdir ()) > * Done in _unlink_nt through NtOpenFile with flags FILE_DIRECTORY_FILE > and FILE_NON_DIRECTORY_FILE which will return STATUS_NOT_A_DIRECTORY > and STATUS_FILE_IS_A_DIRECTORY respectively. > > Have I missed something else? > > Also, I think it's better to have isdev_dev (fh->dev ()) return EROFS, > which is the same as unlink. No, isdev_dev in rmdir returns ENOTEMPTY because /dev is a merge between a virtual and a real directory. You can write into that directory by adding new entries, like the symlinks for stdin/stdout, etc., but of course it's a non-empty dir. It's a kind of stop-gap measure so /dev doesn't get removed accidentally. In retrospect, checking against isdev_dev is a bit unclean here. It would be cleaner to add a method fhandler_dev::rmdir to override the rmdir method of the underlying fhandler_disk_file class and handle this in the fhandler class as desired. I pushed a matching patch. Corinna
Re: [PATCH 08/11] path.cc: Allow to skip filesystem checks
On Jan 18 18:15, Ben wrote: > > > On 18-01-2021 12:36, Corinna Vinschen via Cygwin-patches wrote: > > On Jan 15 14:45, Ben Wijen wrote: > > > > Without any code setting the flag, this doesn't seem to make any > > sense. At least the commit message should reflect on the reasons > > for this change. > > > Something like this: > path.cc: Allow to skip filesystem checks > > When file attributes are of no concern, there is no point to query them. > This can greatly speedup code which doesn't need it. > > For example, this can be used to try a path without filesystem checks > first > and try again with filesystem checks > > > If you want, I can also squash some of these related commits. That's not necessary, but a log msg hint along the lines of "in preparation of " would help. However, just to be clear here. While I really appreciate your efforts, I'm not sure yet if we really *can* skip the filesystem checks. This is dangerous but often perpetrated territory. Trying to work around certain checks almost always resulted in other problems in the past, like POSIX/Linux incompatibilities or forgetting to handle Windows quirks. A certain callousness and high frustration barrier are required. Thanks, Corinna
Re: [PATCH] Cygwin: pty: Set input_available_event only for cygwin pipe.
On Jan 19 00:00, Takashi Yano via Cygwin-patches wrote: > Hi Corinna, > > On Fri, 15 Jan 2021 18:26:31 +0900 > Takashi Yano wrote: > > - cat exits immediately in the following senario. > > 1) Execute env CYGWIN=disable_pcon script > > 2) Execute cmd.exe > > 3) Execute cat in cmd.exe. > > This is caused by setting input_available_event for the pipe for > > non-cygwin app. This patch fixes the issue. > > --- > > winsup/cygwin/fhandler_tty.cc | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc > > index e4993bf31..0b9901974 100644 > > --- a/winsup/cygwin/fhandler_tty.cc > > +++ b/winsup/cygwin/fhandler_tty.cc > > @@ -394,7 +394,8 @@ fhandler_pty_master::accept_input () > > } > > } > > > > - SetEvent (input_available_event); > > + if (write_to == get_output_handle ()) > > +SetEvent (input_available_event); > >ReleaseMutex (input_mutex); > >return ret; > > } > > -- > > 2.30.0 > > > > I would be happy if you could review this patch as well. Sorry, I missed that one! Pushed. Thanks, Corinna
Re: [PATCH 05/11] Cygwin: Move post-dir unlink check
On Jan 18 15:31, Ben wrote: > On 18-01-2021 12:08, Corinna Vinschen via Cygwin-patches wrote: > > On Jan 15 14:45, Ben Wijen wrote: > >> Move post-dir unlink check from > >> fhandler_disk_file::rmdir to _unlink_nt > > > > Why? It's not much of a problem, codewise, but the commit message > > could be improved here. > > > Something like this? > Cygwin: Move post-dir unlink check > > Move post-dir unlink check from > fhandler_disk_file::rmdir to _unlink_nt > > This helps in two ways: > * Now all checks are in one place > * Even if a directory is removed through > _unlink_nt, but not rmdir, the return > value can be trusted. Sure, looks good. You don't have to cramp the text into the first 40 cols, 80 is fine. Thanks, Corinna
