Re: [Chicken-hackers] [PATCH] Fix #765 and a small can of worms related to error handling under Windows
On Thu, Nov 28, 2013 at 09:11:15PM +0100, Peter Bex wrote: On Fri, Nov 22, 2013 at 09:43:31PM +0100, Peter Bex wrote: The error handling in Windows was completely broken; the errmap loop updated map, but looked at errmap. This whole error handling shit needs to get reworked to make the Windows error-errno conversion used everywhere. Winsock2 only *looks* like the BSD sockets API, it uses the same function names but its error handling is completely different so it's not really compatible. So my advice for now is NOT to apply these two patches yet, but rework them in a complete overhaul of Windows error handling. That naturally means that the stability branch should NOT get any of these patches, because the changes will just be too invasive. I was wrong: this can't be generalised. Winsock errors are completely(?) distinct from other errors. Luckily this means that the process patch can be applied as-is, without any changes and it can still go into stability. I'll post a new patch for tcp, hopefully soon. Cheers, Peter -- http://www.more-magic.net ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Re: [Chicken-hackers] [PATCH] Fix #566 and simplify/improve flonum printing code
Hi Peter, Peter Bex peter@xs4all.nl writes: In order to fix #566, I decided it's much easier to rip out the HAVE_GCVT definition in Cygwin. After testing on several platforms, it turns out that gcvt() is really not required, and it's deprecated by POSIX as well. So we should probably stop using it anyway; this also simplifies testing as it doesn't needlessly use different library functions on different platforms. I tried to figure out why HAVE_GCVT was introduced in the first place, but it goes all the way back to the very first commit in the NonGNU CVS even. Felix mentioned that he seemed to recall that it had something to do with making behaviour of the various Windows builds consistent among eachother. However, I've tested all three Windows builds, and they all behave equally well (or better) with s[n]printf() instead of gcvt(). We no longer have a MSVC build, so the original problem probably was there, or it was in an older version of Cygwin/MingW which has been fixed in the meanwhile (it's been over 10 years, so a thing or two will have changed there, as well!). If we ever regain a MSVC build, we can look at restoring this code, but in the meanwhile I prefer the simplicity of this solution. I was even able to get rid of the MINGW hack just below the changed code! wow, thanks going through all that trouble! Shedding some light on dusty corners like that is definitely a good idea to keep the code clean and to improve it, too. I've also taken the opportunity to convert sprintf into a checked snprintf. I'm not 100% sure but I don't think this requires a CVE since you can't easily (at all?) cause over 4096 flonum digits to get printed, and flonum-print-precision is rarely, if ever user-controlled. Feel free to request a CVE if you disagree. I'm not sure about this either. I've pushed the patch though as it looks OK to me and all tests pass, too. I also grepped for gcvt afterwards and couldn't find traces of it anymore. Good riddance! Moritz ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Re: [Chicken-hackers] Ticket 942 - make install uses user's umask
Peter Bex peter@xs4all.nl writes: Thanks guys, for making and testing the patch! Here's a proper git patch w/ changelog. Thanks from me, too. Signed off and pushed! Moritz ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Re: [Chicken-hackers] [PATCH] Fix #566 and simplify/improve flonum printing code
Peter Bex peter@xs4all.nl scripsit: However, I've tested all three Windows builds, and they all behave equally well (or better) with s[n]printf() instead of gcvt(). We no longer have a MSVC build, so the original problem probably was there, As of Visual C++ 2005, Microsoft deprecated all the functions in the C Runtime Library that aren't part of ISO C. So snprintf, which is part of Posix but not ISO C, appears as _snprintf. This is a general problem which we'll have to solve if we ever want a VC++ build. In some cases, like gcvt, the original function survives in deprecated form, but _gcvt is recommended. This is all in the name of avoiding namespace pollution, a silly concern on Windows with its monstrous windows.h file. In both GNU libc and newlib, gcvt just calls sprintf. I've also taken the opportunity to convert sprintf into a checked snprintf. I'm not 100% sure but I don't think this requires a CVE since you can't easily (at all?) cause over 4096 flonum digits to get printed, It would be nonsense to do so anyway: #;1 (parameterize ((flonum-print-precision 4095)) (print 1.1)) 1.100088817841970012523233890533447265625 All those low-order digits represent the triumph of precision over accuracy. I suppose some day we could integrate MPFR into the numbers egg, though. -- John Cowanco...@ccil.orghttp://ccil.org/~cowan Rather than making ill-conceived suggestions for improvement based on uninformed guesses about established conventions in a field of study with which familiarity is limited, it is sometimes better to stick to merely observing the usage and listening to the explanations offered, inserting only questions as needed to fill in gaps in understanding. --Peter Constable ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Re: [Chicken-hackers] [PATCH] Fix #566 and simplify/improve flonum printing code
On Sun, Dec 01, 2013 at 11:31:31AM -0500, John Cowan wrote: Peter Bex peter@xs4all.nl scripsit: However, I've tested all three Windows builds, and they all behave equally well (or better) with s[n]printf() instead of gcvt(). We no longer have a MSVC build, so the original problem probably was there, As of Visual C++ 2005, Microsoft deprecated all the functions in the C Runtime Library that aren't part of ISO C. So snprintf, which is part of Posix but not ISO C, appears as _snprintf. This is a general problem which we'll have to solve if we ever want a VC++ build. In some cases, like gcvt, the original function survives in deprecated form, but _gcvt is recommended. This is all in the name of avoiding namespace pollution, a silly concern on Windows with its monstrous windows.h file. We'll fix that when we get to it. I think there may be other calls like it which don't work as-is. We could fix it in chicken.h by aliasing it to C_snprintf or do it some other way. Cheers, Peter -- http://www.more-magic.net ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Re: [Chicken-hackers] [PATCH] Fix nonblocking socket behaviour on Windows
On Tue, Nov 26, 2013 at 01:24:55AM +, Mario Domenech Goulart wrote:
Unfortunately it seems to break something on XP. chicken-install gets a
TCP timeout error instantly
Without this patch, I don't get the TCP timeout error.
Thanks to some pointers by rivo on IRC, and digging through the
MSDN docs, I've finally made a patch that fixes this stuff properly.
I also noticed whole swaths of code in posixwin.scm which weren't
used at all (so I deleted that), and decided to remove all support
for Winsock 1. Winsock2 dates back to Windows 95, which we don't
support anymore. Anything more recent than that is shipped with
Winsock 2 built-in.
Of course the patch is so complicated by now that it most definitely
should *NOT* go into stability. But the process* fix should go in,
like I said in the other thread.
Question: should we rip out support for pre-NT versions of Windows
altogether? I'm in favor, that would allow us to simplify
##sys#shell-command a bit, too (because get_shlcmd is pretty
complicated and checks with sysinfo() whether it's running on NT).
Cheers,
Peter
--
http://www.more-magic.net
From 57f7a2802b8cf30776387bec857c92dc25579bc2 Mon Sep 17 00:00:00 2001
From: Peter Bex peter@xs4all.nl
Date: Wed, 20 Nov 2013 23:05:40 +0100
Subject: [PATCH 1/2] Several Windows-related fixes and one race
condition-related fix for TCP.
- Fix nonblocking socket behaviour on Windows by actually marking it
nonblocking.
- Fix socket error handling in Windows by using WSAGetLastError() instead of
checking errno.
- Declare tcp should run with interrupts disabled, to prevent race conditions
between multiple threads causing TCP errors (or on UNIX, causing any error
which may overwrite errno).
---
NEWS|2 +
tcp.scm | 199 +--
2 files changed, 118 insertions(+), 83 deletions(-)
diff --git a/NEWS b/NEWS
index a168975..ee5a1e6 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,8 @@
- Export file-type from the posix unit (thanks to Alan Post).
- unsetenv has been fixed on Windows.
- The process procedure has been fixed on Windows.
+ - Nonblocking behaviour on sockets has been fixed on Windows.
+ - Possible race condition while handling TCP errors has been fixed.
- The posix unit will no longer hang upon any error in Windows.
- Platform support
diff --git a/tcp.scm b/tcp.scm
index bba60c4..7bf49a5 100644
--- a/tcp.scm
+++ b/tcp.scm
@@ -28,11 +28,11 @@
(declare
(unit tcp)
(uses extras scheduler)
+ (disable-interrupts) ; Avoid race conditions around errno/WSAGetLastError
(export tcp-close tcp-listen tcp-connect tcp-accept tcp-accept-ready?
##sys#tcp-port-fileno tcp-listener? tcp-addresses
tcp-abandon-port tcp-listener-port tcp-listener-fileno
tcp-port-numbers tcp-buffer-size
tcp-read-timeout tcp-write-timeout tcp-accept-timeout
tcp-connect-timeout)
(foreign-declare #EOF
-#include errno.h
#ifdef _WIN32
# if (defined(HAVE_WINSOCK2_H) defined(HAVE_WS2TCPIP_H))
# include winsock2.h
@@ -41,21 +41,50 @@
# include winsock.h
# endif
/* Beware: winsock2.h must come BEFORE windows.h */
-# define socklen_t int
+# define socklen_t int
static WSADATA wsa;
-# define fcntl(a, b, c) 0
-# ifndef EWOULDBLOCK
-# define EWOULDBLOCK 0
+# ifndef SHUT_RD
+# define SHUT_RDSD_RECEIVE
# endif
-# ifndef EINPROGRESS
-# define EINPROGRESS 0
-# endif
-# ifndef EAGAIN
-# define EAGAIN 0
+# ifndef SHUT_WR
+# define SHUT_WRSD_SEND
# endif
+
# define typecorrect_getsockopt(socket, level, optname, optval, optlen)
\
getsockopt(socket, level, optname, (char *)optval, optlen)
+
+static C_word make_socket_nonblocking (C_word sock) {
+ int fd = C_unfix(sock);
+ C_return(C_mk_bool(ioctlsocket(fd, FIONBIO, (void *)fd) != SOCKET_ERROR)) ;
+}
+
+/* This is a bit of a hack, but it keeps things simple */
+static C_TLS char *last_wsa_errorstring = NULL;
+
+static char *errormsg_from_code(int code) {
+ int bufsize;
+ if (last_wsa_errorstring != NULL) {
+LocalFree(last_wsa_errorstring);
+last_wsa_errorstring = NULL;
+ }
+ bufsize = FormatMessage(
+ FORMAT_MESSAGE_ALLOCATE_BUFFER |
+ FORMAT_MESSAGE_FROM_SYSTEM |
+ FORMAT_MESSAGE_IGNORE_INSERTS,
+ NULL, code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+ (LPTSTR) last_wsa_errorstring, 0, NULL);
+ if (bufsize == 0) return ERROR WHILE FETCHING ERROR;
+ return last_wsa_errorstring;
+}
+
+# define get_last_socket_error() WSAGetLastError()
+# define should_retry_call() (WSAGetLastError() == WSAEWOULDBLOCK)
+/* Not EINPROGRESS in winsock. Nonblocking connect returns EWOULDBLOCK... */
+# define call_in_progress() (WSAGetLastError() == WSAEWOULDBLOCK)
+# define call_was_interrupted() (WSAGetLastError() == WSAEINTR) /* ? */
+
#else
+# include errno.h
# include fcntl.h
# include sys/socket.h
# include sys/time.h
@@ -64,12 +93,22 @@ static WSADATA wsa;
# include
Re: [Chicken-hackers] [PATCH] Fix nonblocking socket behaviour on Windows
Peter Bex scripsit: Question: should we rip out support for pre-NT versions of Windows altogether? I'm in favor, that would allow us to simplify ##sys#shell-command a bit, too (because get_shlcmd is pretty complicated and checks with sysinfo() whether it's running on NT). IMO we can do better than that. The last release of the Win95 family, WinME, was EOLed more than seven years ago. Windows NT 4.0 was EOLed almost nine years ago because of unfixable security flaws. Windows 2000 was EOLed more than three years ago. According to Net Applications best guesses, pre-XP versions represent about one in a thousand desktops out there. We should simply drop support for everything except XP (34%), Vista (4%), Win 7 (50%), and Win 8 (10%). (Do we support Windows 8 yet?) That is equivalent to saying the internal version number must be greater than 5.0. -- Do what you will, John Cowan this Life's a Fictionco...@ccil.org And is made up of http://www.ccil.org/~cowan Contradiction. --William Blake ___ Chicken-hackers mailing list Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers
