[PATCH] Cygwin: fchmodat: add limited support for AT_SYMLINK_NOFOLLOW
Allow fchmodat with the AT_SYMLINK_NOFOLLOW flag to succeed on
non-symlinks. Previously it always failed, as it does on Linux. But
POSIX permits it to succeed on non-symlinks even if it fails on
symlinks.
The reason for following POSIX rather than Linux is to make gnulib
report that fchmodat works on Cygwin. This improves the efficiency of
packages like GNU tar that use gnulib's fchmodat module. Previously
such packages would use a gnulib replacement for fchmodat on Cygwin.
---
winsup/cygwin/syscalls.cc | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 4cc8d07f5..0983cc76a 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4787,17 +4787,27 @@ fchmodat (int dirfd, const char *pathname, mode_t mode,
int flags)
tmp_pathbuf tp;
__try
{
- if (flags)
+ if (flags & ~AT_SYMLINK_NOFOLLOW)
{
- /* BSD has lchmod, but Linux does not. POSIX says
-AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks; but Linux
-blindly fails even for non-symlinks. */
- set_errno ((flags & ~AT_SYMLINK_NOFOLLOW) ? EINVAL : EOPNOTSUPP);
+ set_errno (EINVAL);
__leave;
}
char *path = tp.c_get ();
if (gen_full_path_at (path, dirfd, pathname))
__leave;
+ if (flags)
+ {
+ /* BSD has lchmod, but Linux does not. POSIX says
+AT_SYMLINK_NOFOLLOW is allowed to fail on symlinks.
+Linux blindly fails even for non-symlinks, but we allow
+it to succeed. */
+ path_conv pc (path);
+ if (pc.issymlink ())
+ {
+ set_errno (EOPNOTSUPP);
+ __leave;
+ }
+ }
return chmod (path, mode);
}
__except (EFAULT) {}
--
2.30.0
[PATCH v6 4/4] Cygwin: pty: Allow multiple apps to enable pseudo console simultaneously.
- After commit bb428520, there has been the disadvantage:
7) Pseudo console cannot be activated if it is already activated for
another process on same pty.
This patch clears this disadvantage.
---
winsup/cygwin/fhandler.h | 7 +-
winsup/cygwin/fhandler_tty.cc | 344 ++
winsup/cygwin/select.cc | 4 +-
winsup/cygwin/spawn.cc| 44 +++--
winsup/cygwin/tty.cc | 2 +-
winsup/cygwin/tty.h | 6 +-
6 files changed, 314 insertions(+), 93 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 0cbc18877..b1f3150ff 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2274,6 +2274,9 @@ class fhandler_pty_common: public fhandler_termios
}
void resize_pseudo_console (struct winsize *);
+ static DWORD get_console_process_id (DWORD pid, bool match,
+ bool cygwin = false,
+ bool stub_only = false);
protected:
static BOOL process_opost_output (HANDLE h, const void *ptr, ssize_t& len,
@@ -2353,14 +2356,14 @@ class fhandler_pty_slave: public fhandler_pty_common
bool term_has_pcon_cap (const WCHAR *env);
void set_switch_to_pcon (void);
void reset_switch_to_pcon (void);
- void mask_switch_to_pcon_in (bool mask);
+ void mask_switch_to_pcon_in (bool mask, bool xfer);
void setup_locale (void);
tty *get_ttyp () { return (tty *) tc (); } /* Override as public */
void create_invisible_console (void);
static void transfer_input (xfer_dir dir, HANDLE from, tty *ttyp,
_minor_t unit, HANDLE input_available_event);
HANDLE get_input_available_event (void) { return input_available_event; }
- bool pcon_activated (void) { return get_ttyp ()->h_pseudo_console; }
+ bool pcon_activated (void) { return get_ttyp ()->pcon_activated; }
};
#define __ptsname(buf, unit) __small_sprintf ((buf), "/dev/pty%d", (unit))
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 6c88e1abf..8c875efc1 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -64,8 +64,9 @@ struct pipe_reply {
extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */
-static DWORD
-get_console_process_id (DWORD pid, bool match)
+DWORD
+fhandler_pty_common::get_console_process_id (DWORD pid, bool match,
+bool cygwin, bool stub_only)
{
tmp_pathbuf tp;
DWORD *list = (DWORD *) tp.c_get ();
@@ -75,16 +76,34 @@ get_console_process_id (DWORD pid, bool match)
if (num == 0 || num > buf_size)
return 0;
- DWORD res = 0;
+ DWORD res_pri = 0, res = 0;
/* Last one is the oldest. */
/* https://github.com/microsoft/terminal/issues/95 */
for (int i = (int) num - 1; i >= 0; i--)
if ((match && list[i] == pid) || (!match && list[i] != pid))
{
- res = list[i];
- break;
+ if (!cygwin)
+ {
+ res_pri = list[i];
+ break;
+ }
+ else
+ {
+ pinfo p (cygwin_pid (list[i]));
+ if (!!p && p->dwProcessId && p->exec_dwProcessId
+ && p->dwProcessId != p->exec_dwProcessId)
+ {
+ res_pri = list[i];
+ break;
+ }
+ if (!!p && !res)
+ res = list[i];
+ }
}
- return res;
+ if (stub_only)
+return res_pri;
+ else
+return res_pri ?: res;
}
static bool isHybrid;
@@ -938,7 +957,7 @@ fhandler_pty_slave::init (HANDLE h, DWORD a, mode_t)
void
fhandler_pty_slave::set_switch_to_pcon (void)
{
- if (!get_ttyp ()->switch_to_pcon_in)
+ if (!isHybrid)
{
isHybrid = true;
setup_locale ();
@@ -965,9 +984,10 @@ fhandler_pty_slave::reset_switch_to_pcon (void)
{
CloseHandle (h_gdb_process);
h_gdb_process = NULL;
- if (isHybrid && get_ttyp ()->pcon_pid == myself->pid)
+ if (isHybrid)
{
- if (get_ttyp ()->switch_to_pcon_in)
+ if (get_ttyp ()->switch_to_pcon_in
+ && get_ttyp ()->pcon_pid == myself->pid)
fhandler_pty_slave::transfer_input (fhandler_pty_slave::to_cyg,
get_handle (),
get_ttyp (), get_minor (),
@@ -977,7 +997,7 @@ fhandler_pty_slave::reset_switch_to_pcon (void)
the console of the parent process will fail.
Therefore, never close pseudo console here. */
return;
- bool need_restore_handles = !!get_ttyp ()->h_pseudo_console;
+ bool need_restore_handles = get_ttyp ()->pcon_activated;
close_pseudoconsole (get_ttyp ());
if (need_restore_handles)
{
@@ -1028,11 +1048,8 @@ fhandler_pty_slave::reset_switch_to_pcon (void)
if (fix_err)
[PATCH v6 3/4] Cygwin: pty: Make apps using console APIs be able to debug with gdb.
- After commit bb428520, there has been the disadvantage:
2) The apps which use console API cannot be debugged with gdb. This
is because pseudo console is not activated since gdb uses
CreateProcess() rather than exec(). Even with this limitation,
attaching gdb to native app, in which pseudo console is already
activated, works.
This patch clears this disadvantage.
---
winsup/cygwin/fhandler.h | 4 +-
winsup/cygwin/fhandler_tty.cc | 294 ++
winsup/cygwin/select.cc | 5 +-
winsup/cygwin/spawn.cc| 2 +
winsup/cygwin/tty.cc | 7 +-
winsup/cygwin/tty.h | 5 +-
6 files changed, 277 insertions(+), 40 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 378e9c13b..0cbc18877 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2235,13 +2235,14 @@ class fhandler_pty_common: public fhandler_termios
fhandler_pty_common ()
: fhandler_termios (),
output_mutex (NULL), input_mutex (NULL),
-input_available_event (NULL)
+input_available_event (NULL), slave_reading (NULL)
{
pc.file_attributes (FILE_ATTRIBUTE_NORMAL);
}
static const unsigned pipesize = 128 * 1024;
HANDLE output_mutex, input_mutex;
HANDLE input_available_event;
+ HANDLE slave_reading;
bool use_archetype () const {return true;}
DWORD __acquire_output_mutex (const char *fn, int ln, DWORD ms);
@@ -2359,6 +2360,7 @@ class fhandler_pty_slave: public fhandler_pty_common
static void transfer_input (xfer_dir dir, HANDLE from, tty *ttyp,
_minor_t unit, HANDLE input_available_event);
HANDLE get_input_available_event (void) { return input_available_event; }
+ bool pcon_activated (void) { return get_ttyp ()->h_pseudo_console; }
};
#define __ptsname(buf, unit) __small_sprintf ((buf), "/dev/pty%d", (unit))
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 34cff2ae5..6c88e1abf 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -27,6 +27,7 @@ details. */
#include "cygwait.h"
#include "registry.h"
#include "tls_pbuf.h"
+#include "winf.h"
#ifndef PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE
#define PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE 0x00020016
@@ -87,21 +88,47 @@ get_console_process_id (DWORD pid, bool match)
}
static bool isHybrid;
+static HANDLE h_gdb_process;
static void
-set_switch_to_pcon (HANDLE h)
+set_switch_to_pcon (HANDLE *in, HANDLE *out, HANDLE *err, bool iscygwin)
{
cygheap_fdenum cfd (false);
int fd;
+ fhandler_base *replace_in = NULL, *replace_out = NULL, *replace_err = NULL;
+ fhandler_pty_slave *ptys_pcon = NULL;
while ((fd = cfd.next ()) >= 0)
-if (cfd->get_major () == DEV_PTYS_MAJOR)
- {
- fhandler_base *fh = cfd;
- fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
- if (h == ptys->get_handle ())
- ptys->set_switch_to_pcon ();
- return;
- }
+{
+ if (*in == cfd->get_handle () ||
+ (fd == 0 && *in == GetStdHandle (STD_INPUT_HANDLE)))
+ replace_in = (fhandler_base *) cfd;
+ if (*out == cfd->get_output_handle () ||
+ (fd == 1 && *out == GetStdHandle (STD_OUTPUT_HANDLE)))
+ replace_out = (fhandler_base *) cfd;
+ if (*err == cfd->get_output_handle () ||
+ (fd == 2 && *err == GetStdHandle (STD_ERROR_HANDLE)))
+ replace_err = (fhandler_base *) cfd;
+ if (cfd->get_major () == DEV_PTYS_MAJOR)
+ {
+ fhandler_base *fh = cfd;
+ fhandler_pty_slave *ptys = (fhandler_pty_slave *) fh;
+ if (*in == ptys->get_handle ())
+ ptys_pcon = ptys;
+ }
+}
+ if (!iscygwin && ptys_pcon)
+ptys_pcon->set_switch_to_pcon ();
+ if (replace_in)
+{
+ if (iscygwin && ptys_pcon->pcon_activated ())
+ *in = replace_in->get_handle_cyg ();
+ else
+ *in = replace_in->get_handle ();
+}
+ if (replace_out)
+*out = replace_out->get_output_handle ();
+ if (replace_err)
+*err = replace_err->get_output_handle ();
}
#define DEF_HOOK(name) static __typeof__ (name) *name##_Orig
@@ -115,16 +142,55 @@ CreateProcessA_Hooked
BOOL inh, DWORD f, LPVOID e, LPCSTR d,
LPSTARTUPINFOA si, LPPROCESS_INFORMATION pi)
{
- HANDLE h;
- if (!isHybrid)
+ STARTUPINFOEXA siex = {0, };
+ if (si->cb == sizeof (STARTUPINFOEXA))
+siex = *(STARTUPINFOEXA *)si;
+ else
+siex.StartupInfo = *si;
+ STARTUPINFOA *siov =
+ if (!(si->dwFlags & STARTF_USESTDHANDLES))
{
- if (si->dwFlags & STARTF_USESTDHANDLES)
- h = si->hStdInput;
- else
- h = GetStdHandle (STD_INPUT_HANDLE);
- set_switch_to_pcon (h);
+ siov->dwFlags |= STARTF_USESTDHANDLES;
+ siov->hStdInput = GetStdHandle (STD_INPUT_HANDLE);
+ siov->hStdOutput = GetStdHandle (STD_OUTPUT_HANDLE);
+ siov->hStdError = GetStdHandle (STD_ERROR_HANDLE);
}
- return
[PATCH v6 2/4] Cygwin: pty: Keep code page between non-cygwin apps.
- After commit bb428520, there has been the disadvantage:
4) Code page cannot be changed by chcp.com. Acctually, chcp works
itself and changes code page of its own pseudo console. However,
since pseudo console is recreated for another process, it cannot
inherit the code page.
This patch clears this disadvantage.
---
winsup/cygwin/fhandler_tty.cc | 7 +++
winsup/cygwin/tty.cc | 2 ++
winsup/cygwin/tty.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index f0b2cd60a..34cff2ae5 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -2845,6 +2845,11 @@ fhandler_pty_slave::setup_pseudoconsole (bool nopcon)
HPCON_INTERNAL *hp = (HPCON_INTERNAL *) get_ttyp ()->h_pseudo_console;
get_ttyp ()->h_pcon_write_pipe = hp->hWritePipe;
}
+
+ if (get_ttyp ()->previous_code_page)
+SetConsoleCP (get_ttyp ()->previous_code_page);
+ if (get_ttyp ()->previous_output_code_page)
+SetConsoleOutputCP (get_ttyp ()->previous_output_code_page);
return true;
cleanup_pcon_in:
@@ -2884,6 +2889,8 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp)
if (ttyp->h_pseudo_console)
{
ttyp->wait_pcon_fwd ();
+ ttyp->previous_code_page = GetConsoleCP ();
+ ttyp->previous_output_code_page = GetConsoleOutputCP ();
FreeConsole ();
AttachConsole (ATTACH_PARENT_PROCESS);
HPCON_INTERNAL *hp = (HPCON_INTERNAL *) ttyp->h_pseudo_console;
diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
index 436f5c6c3..908166a37 100644
--- a/winsup/cygwin/tty.cc
+++ b/winsup/cygwin/tty.cc
@@ -246,6 +246,8 @@ tty::init ()
has_csi6n = false;
need_invisible_console = false;
invisible_console_pid = 0;
+ previous_code_page = 0;
+ previous_output_code_page = 0;
}
HANDLE
diff --git a/winsup/cygwin/tty.h b/winsup/cygwin/tty.h
index eb604588c..061357437 100644
--- a/winsup/cygwin/tty.h
+++ b/winsup/cygwin/tty.h
@@ -107,6 +107,8 @@ private:
bool has_csi6n;
bool need_invisible_console;
pid_t invisible_console_pid;
+ UINT previous_code_page;
+ UINT previous_output_code_page;
public:
HANDLE from_master () const { return _from_master; }
--
2.30.0
[PATCH v6 1/4] Cygwin: pty: Inherit typeahead data between two input pipes.
- PTY has a problem that the key input, which is typed during windows
native app is running, disappears when it returns to shell. This is
beacuse pty has two input pipes, one is for cygwin apps and the other
one is for native windows apps. The key input during windows native
program is running is sent to the second input pipe while cygwin
shell reads input from the first input pipe. This issue had been
fixed once by commit 29431fcb, however, the new implementation of
pseudo console support by commit bb428520 could not inherit this
feature. This patch realize transfering input data between these
two pipes bidirectionally by utilizing cygwin-console-helper process.
The helper process is launched prior to starting the non-cygwin app,
however, exits immediately unlike previous implementation.
---
winsup/cygwin/fhandler.h | 14 +-
winsup/cygwin/fhandler_tty.cc | 545 +++---
winsup/cygwin/spawn.cc| 82 +++--
winsup/cygwin/tty.cc | 2 -
winsup/cygwin/tty.h | 8 +-
5 files changed, 504 insertions(+), 147 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index af1ef3a45..378e9c13b 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2291,6 +2291,13 @@ class fhandler_pty_slave: public fhandler_pty_common
void fch_close_handles ();
public:
+ /* Transfer direction for transfer_input() */
+ enum xfer_dir
+ {
+to_nat,
+to_cyg
+ };
+
/* Constructor */
fhandler_pty_slave (int);
@@ -2340,7 +2347,7 @@ class fhandler_pty_slave: public fhandler_pty_common
copyto (fh);
return fh;
}
- bool setup_pseudoconsole (STARTUPINFOEXW *si, bool nopcon);
+ bool setup_pseudoconsole (bool nopcon);
static void close_pseudoconsole (tty *ttyp);
bool term_has_pcon_cap (const WCHAR *env);
void set_switch_to_pcon (void);
@@ -2349,6 +2356,9 @@ class fhandler_pty_slave: public fhandler_pty_common
void setup_locale (void);
tty *get_ttyp () { return (tty *) tc (); } /* Override as public */
void create_invisible_console (void);
+ static void transfer_input (xfer_dir dir, HANDLE from, tty *ttyp,
+ _minor_t unit, HANDLE input_available_event);
+ HANDLE get_input_available_event (void) { return input_available_event; }
};
#define __ptsname(buf, unit) __small_sprintf ((buf), "/dev/pty%d", (unit))
@@ -2361,6 +2371,8 @@ public:
HANDLE from_master_cyg;
HANDLE to_master;
HANDLE to_master_cyg;
+HANDLE to_slave;
+HANDLE to_slave_cyg;
HANDLE master_ctl;
HANDLE input_available_event;
};
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 7f0752614..f0b2cd60a 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -56,6 +56,8 @@ struct pipe_reply {
HANDLE from_master_cyg;
HANDLE to_master;
HANDLE to_master_cyg;
+ HANDLE to_slave;
+ HANDLE to_slave_cyg;
DWORD error;
};
@@ -848,8 +850,14 @@ fhandler_pty_slave::reset_switch_to_pcon (void)
return;
if (isHybrid)
return;
+ if (get_ttyp ()->switch_to_pcon_in && !get_ttyp ()->h_pseudo_console)
+fhandler_pty_slave::transfer_input (fhandler_pty_slave::to_cyg,
+ get_handle (),
+ get_ttyp (), get_minor (),
+ input_available_event);
get_ttyp ()->pcon_pid = 0;
get_ttyp ()->switch_to_pcon_in = false;
+ get_ttyp ()->h_pseudo_console = NULL;
}
ssize_t __stdcall
@@ -891,6 +899,22 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
void
fhandler_pty_slave::mask_switch_to_pcon_in (bool mask)
{
+ if (get_ttyp ()->switch_to_pcon_in
+ && (get_ttyp ()->pcon_pid == myself->pid
+ || !get_ttyp ()->h_pseudo_console)
+ && get_ttyp ()->mask_switch_to_pcon_in != mask)
+{
+ if (mask)
+ fhandler_pty_slave::transfer_input (fhandler_pty_slave::to_cyg,
+ get_handle (),
+ get_ttyp (), get_minor (),
+ input_available_event);
+ else
+ fhandler_pty_slave::transfer_input (fhandler_pty_slave::to_nat,
+ get_handle_cyg (),
+ get_ttyp (), get_minor (),
+ input_available_event);
+}
get_ttyp ()->mask_switch_to_pcon_in = mask;
}
@@ -1591,7 +1615,7 @@ fhandler_pty_common::resize_pseudo_console (struct
winsize *ws)
size.X = ws->ws_col;
size.Y = ws->ws_row;
pinfo p (get_ttyp ()->pcon_pid);
- if (p && !get_ttyp ()->do_not_resize_pcon)
+ if (p)
{
HPCON_INTERNAL hpcon_local;
HANDLE pcon_owner =
@@ -1763,6 +1787,17 @@ fhandler_pty_master::write (const void *ptr, size_t len)
get_ttyp ()->pcon_start = false;
}
[PATCH v6 0/4] Improve pseudo console support.
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). 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 | 23 +- winsup/cygwin/fhandler_tty.cc | 1120 +++-- winsup/cygwin/select.cc |7 +- winsup/cygwin/spawn.cc| 106 +++- winsup/cygwin/tty.cc | 13 +- winsup/cygwin/tty.h | 21 +- 6 files changed, 1058 insertions(+), 232 deletions(-) -- 2.30.0
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 v5 0/4] Improve pseudo console support.
On Tue, 26 Jan 2021 12:52:25 +0900 Takashi Yano 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. I have another fix for v5. Please wait for v6. -- Takashi Yano
