| Message ID | 20170620163057.6272-1-aserdean@cloudbasesolutions.com |
|---|---|
| State | Accepted |
| Headers | show |
On 20 June 2017 at 09:31, Alin Serdean <aserdean@cloudbasesolutions.com> wrote: > A lot of tests are failing, due to the open flow ports being outputted > using > names instead of numbers. > i.e.: http://64.119.130.115/ovs/beb75a40fdc295bfd6521b0068b4cd > 12f6de507c/testsuite.dir/0464/testsuite.log.gz > > The issues encountered above is because 'monitor' with 'detach' arguments > are > specified, that in turn will call 'close_standard_fds' > (https://github.com/openvswitch/ovs/blob/master/lib/daemon-unix.c#L472) > which will create a duplicate fd over '/dev/null' on Linux and 'nul' on > Windows. > > 'isatty' will be called on those FDs. > What POSIX standard says: > http://pubs.opengroup.org/onlinepubs/009695399/functions/isatty.html > 'The isatty() function shall test whether fildes, an open file descriptor, > is associated with a terminal device.' > What MSDN says: > https://msdn.microsoft.com/en-us/library/f4s0ddew(VS.80).aspx > 'The _isatty function determines whether fd is associated with a character > device (a terminal, console, printer, or serial port).' > > This patch adds another check using 'GetConsoleMode' > https://msdn.microsoft.com/en-us/library/windows/desktop/ > ms683167(v=vs.85).aspx > which will fail if the handle pointing to the file descriptor is not > associated > to a console. > > Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> > Co-authroed-by: Ben Pfaff <blp@ovn.org> > Ben, Do I have take the liberty to add your Signed-off-by? > --- > v2 replace `isatty` in unistd.h > --- > include/windows/unistd.h | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/include/windows/unistd.h b/include/windows/unistd.h > index 2e9f0ae..21cc56f 100644 > --- a/include/windows/unistd.h > +++ b/include/windows/unistd.h > @@ -85,4 +85,20 @@ __inline long sysconf(int type) > return value; > } > > +/* On Windows, a console is a specialized character device, and isatty() > only > + * reports whether a file description is a character device and thus > reports > + * that devices such as /dev/null are ttys. This replacement avoids that > + * problem. */ > +#undef isatty > +#define isatty(fd) rpl_isatty(fd) > +static __inline int > +rpl_isatty(int fd) > +{ > + HANDLE h = (HANDLE) _get_osfhandle(fd); > + DWORD st; > + return (_isatty(STDOUT_FILENO) > + && h != INVALID_HANDLE_VALUE > + && GetConsoleMode(h, &st)); > +} > + > #endif /* unistd.h */ > -- > 2.10.2.windows.1 > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Sorry about the delay, everyone. I was traveling and not paying much attention to email. I applied this to master. On Tue, Jun 20, 2017 at 11:11:05AM -0700, Guru Shetty wrote: > On 20 June 2017 at 09:31, Alin Serdean <aserdean@cloudbasesolutions.com> > wrote: > > > A lot of tests are failing, due to the open flow ports being outputted > > using > > names instead of numbers. > > i.e.: http://64.119.130.115/ovs/beb75a40fdc295bfd6521b0068b4cd > > 12f6de507c/testsuite.dir/0464/testsuite.log.gz > > > > The issues encountered above is because 'monitor' with 'detach' arguments > > are > > specified, that in turn will call 'close_standard_fds' > > (https://github.com/openvswitch/ovs/blob/master/lib/daemon-unix.c#L472) > > which will create a duplicate fd over '/dev/null' on Linux and 'nul' on > > Windows. > > > > 'isatty' will be called on those FDs. > > What POSIX standard says: > > http://pubs.opengroup.org/onlinepubs/009695399/functions/isatty.html > > 'The isatty() function shall test whether fildes, an open file descriptor, > > is associated with a terminal device.' > > What MSDN says: > > https://msdn.microsoft.com/en-us/library/f4s0ddew(VS.80).aspx > > 'The _isatty function determines whether fd is associated with a character > > device (a terminal, console, printer, or serial port).' > > > > This patch adds another check using 'GetConsoleMode' > > https://msdn.microsoft.com/en-us/library/windows/desktop/ > > ms683167(v=vs.85).aspx > > which will fail if the handle pointing to the file descriptor is not > > associated > > to a console. > > > > Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> > > Co-authroed-by: Ben Pfaff <blp@ovn.org> > > > Ben, > Do I have take the liberty to add your Signed-off-by? > > > > --- > > v2 replace `isatty` in unistd.h > > --- > > include/windows/unistd.h | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/include/windows/unistd.h b/include/windows/unistd.h > > index 2e9f0ae..21cc56f 100644 > > --- a/include/windows/unistd.h > > +++ b/include/windows/unistd.h > > @@ -85,4 +85,20 @@ __inline long sysconf(int type) > > return value; > > } > > > > +/* On Windows, a console is a specialized character device, and isatty() > > only > > + * reports whether a file description is a character device and thus > > reports > > + * that devices such as /dev/null are ttys. This replacement avoids that > > + * problem. */ > > +#undef isatty > > +#define isatty(fd) rpl_isatty(fd) > > +static __inline int > > +rpl_isatty(int fd) > > +{ > > + HANDLE h = (HANDLE) _get_osfhandle(fd); > > + DWORD st; > > + return (_isatty(STDOUT_FILENO) > > + && h != INVALID_HANDLE_VALUE > > + && GetConsoleMode(h, &st)); > > +} > > + > > #endif /* unistd.h */ > > -- > > 2.10.2.windows.1 > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
No worries, thanks a lot! > -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Thursday, July 6, 2017 6:33 PM > To: Guru Shetty <guru@ovn.org> > Cc: Alin Serdean <aserdean@cloudbasesolutions.com>; > dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v2] Fix nonstandard isatty on Windows > > Sorry about the delay, everyone. I was traveling and not paying much > attention to email. > > I applied this to master. > > On Tue, Jun 20, 2017 at 11:11:05AM -0700, Guru Shetty wrote: > > On 20 June 2017 at 09:31, Alin Serdean > > <aserdean@cloudbasesolutions.com> > > wrote: > > > > > A lot of tests are failing, due to the open flow ports being > > > outputted using names instead of numbers. > > > i.e.: http://64.119.130.115/ovs/beb75a40fdc295bfd6521b0068b4cd > > > 12f6de507c/testsuite.dir/0464/testsuite.log.gz > > > > > > The issues encountered above is because 'monitor' with 'detach' > > > arguments are specified, that in turn will call 'close_standard_fds' > > > (https://github.com/openvswitch/ovs/blob/master/lib/daemon- > unix.c#L4 > > > 72) which will create a duplicate fd over '/dev/null' on Linux and > > > 'nul' on Windows. > > > > > > 'isatty' will be called on those FDs. > > > What POSIX standard says: > > > http://pubs.opengroup.org/onlinepubs/009695399/functions/isatty.html > > > 'The isatty() function shall test whether fildes, an open file > > > descriptor, is associated with a terminal device.' > > > What MSDN says: > > > https://msdn.microsoft.com/en-us/library/f4s0ddew(VS.80).aspx > > > 'The _isatty function determines whether fd is associated with a > > > character device (a terminal, console, printer, or serial port).' > > > > > > This patch adds another check using 'GetConsoleMode' > > > https://msdn.microsoft.com/en-us/library/windows/desktop/ > > > ms683167(v=vs.85).aspx > > > which will fail if the handle pointing to the file descriptor is not > > > associated to a console. > > > > > > Signed-off-by: Alin Gabriel Serdean > > > <aserdean@cloudbasesolutions.com> > > > Co-authroed-by: Ben Pfaff <blp@ovn.org> > > > > > Ben, > > Do I have take the liberty to add your Signed-off-by? > > > > > > > --- > > > v2 replace `isatty` in unistd.h > > > --- > > > include/windows/unistd.h | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/include/windows/unistd.h b/include/windows/unistd.h > > > index 2e9f0ae..21cc56f 100644 > > > --- a/include/windows/unistd.h > > > +++ b/include/windows/unistd.h > > > @@ -85,4 +85,20 @@ __inline long sysconf(int type) > > > return value; > > > } > > > > > > +/* On Windows, a console is a specialized character device, and > > > +isatty() > > > only > > > + * reports whether a file description is a character device and > > > + thus > > > reports > > > + * that devices such as /dev/null are ttys. This replacement > > > +avoids that > > > + * problem. */ > > > +#undef isatty > > > +#define isatty(fd) rpl_isatty(fd) > > > +static __inline int > > > +rpl_isatty(int fd) > > > +{ > > > + HANDLE h = (HANDLE) _get_osfhandle(fd); > > > + DWORD st; > > > + return (_isatty(STDOUT_FILENO) > > > + && h != INVALID_HANDLE_VALUE > > > + && GetConsoleMode(h, &st)); } > > > + > > > #endif /* unistd.h */ > > > -- > > > 2.10.2.windows.1 > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
diff --git a/include/windows/unistd.h b/include/windows/unistd.h index 2e9f0ae..21cc56f 100644 --- a/include/windows/unistd.h +++ b/include/windows/unistd.h @@ -85,4 +85,20 @@ __inline long sysconf(int type) return value; } +/* On Windows, a console is a specialized character device, and isatty() only + * reports whether a file description is a character device and thus reports + * that devices such as /dev/null are ttys. This replacement avoids that + * problem. */ +#undef isatty +#define isatty(fd) rpl_isatty(fd) +static __inline int +rpl_isatty(int fd) +{ + HANDLE h = (HANDLE) _get_osfhandle(fd); + DWORD st; + return (_isatty(STDOUT_FILENO) + && h != INVALID_HANDLE_VALUE + && GetConsoleMode(h, &st)); +} + #endif /* unistd.h */
A lot of tests are failing, due to the open flow ports being outputted using names instead of numbers. i.e.: http://64.119.130.115/ovs/beb75a40fdc295bfd6521b0068b4cd12f6de507c/testsuite.dir/0464/testsuite.log.gz The issues encountered above is because 'monitor' with 'detach' arguments are specified, that in turn will call 'close_standard_fds' (https://github.com/openvswitch/ovs/blob/master/lib/daemon-unix.c#L472) which will create a duplicate fd over '/dev/null' on Linux and 'nul' on Windows. 'isatty' will be called on those FDs. What POSIX standard says: http://pubs.opengroup.org/onlinepubs/009695399/functions/isatty.html 'The isatty() function shall test whether fildes, an open file descriptor, is associated with a terminal device.' What MSDN says: https://msdn.microsoft.com/en-us/library/f4s0ddew(VS.80).aspx 'The _isatty function determines whether fd is associated with a character device (a terminal, console, printer, or serial port).' This patch adds another check using 'GetConsoleMode' https://msdn.microsoft.com/en-us/library/windows/desktop/ms683167(v=vs.85).aspx which will fail if the handle pointing to the file descriptor is not associated to a console. Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Co-authroed-by: Ben Pfaff <blp@ovn.org> --- v2 replace `isatty` in unistd.h --- include/windows/unistd.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)