Re: [Chicken-hackers] [PATCH] Fix #566 and simplify/improve flonum printing code

2013-12-01 Thread Peter Bex
On Sun, Dec 01, 2013 at 11:31:31AM -0500, John Cowan wrote:
> Peter Bex  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  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 #566 and simplify/improve flonum printing code

2013-12-01 Thread John Cowan
Peter Bex  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  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

2013-12-01 Thread Moritz Heidkamp
Hi Peter,

Peter Bex  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


[Chicken-hackers] [PATCH] Fix #566 and simplify/improve flonum printing code

2013-11-18 Thread Peter Bex
Hi all,

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!

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.

Cheers,
Peter
-- 
http://www.more-magic.net
>From beeaaf02c5e675fee031ed5eb856d6c391636e85 Mon Sep 17 00:00:00 2001
From: Peter Bex 
Date: Mon, 18 Nov 2013 22:06:13 +0100
Subject: [PATCH] Remove HAVE_GCVT check and definition.  Use snprintf on all
 platforms.

gcvt() is deprecated in POSIX, and it produces problematic output in
at least cygwin.  For example, -0.0 is printed as "0.0" and more fun
things.  This fix adds a regression test for the negative zero case.

Replace sprintf usage with snprintf just in case someone decides to
use insane values for flonum_print_precision.
---
 Makefile.aix  |1 -
 Makefile.cross-linux-mingw|1 -
 Makefile.cygwin   |1 -
 Makefile.haiku|1 -
 Makefile.hurd |1 -
 Makefile.linux|1 -
 Makefile.mingw|1 -
 Makefile.mingw-msys   |1 -
 Makefile.solaris  |1 -
 chicken.h |1 -
 runtime.c |   17 +++--
 tests/numbers-string-conversion-tests.scm |4 +++-
 12 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/Makefile.aix b/Makefile.aix
index d453499..8bde946 100644
--- a/Makefile.aix
+++ b/Makefile.aix
@@ -97,7 +97,6 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_ALLOCA_H 1" >>$@
echo "#define HAVE_GRP_H 1" >>$@
echo "#define HAVE_ERRNO_H 1" >>$@
-   echo "#define HAVE_GCVT 1" >>$@
echo "#define HAVE_SYSEXITS_H 1" >>$@
echo "#define C_STACK_GROWS_DOWNWARD 1" >>$@
 ifdef GCHOOKS
diff --git a/Makefile.cross-linux-mingw b/Makefile.cross-linux-mingw
index c18dd18..e1778ca 100644
--- a/Makefile.cross-linux-mingw
+++ b/Makefile.cross-linux-mingw
@@ -116,7 +116,6 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_ALLOCA_H 1" >>$@
echo "#define HAVE_DIRECT_H 1" >>$@
echo "#define HAVE_ERRNO_H 1" >>$@
-   echo "#define HAVE_GCVT 1" >>$@
echo "#define HAVE_LOADLIBRARY 1" >>$@
echo "#define HAVE_GETPROCADDRESS 1" >>$@
echo "#define HAVE_WINSOCK2_H 1" >>$@
diff --git a/Makefile.cygwin b/Makefile.cygwin
index b4122ef..1518a16 100644
--- a/Makefile.cygwin
+++ b/Makefile.cygwin
@@ -113,7 +113,6 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_ALLOCA_H 1" >>$@
echo "#define HAVE_GRP_H 1" >>$@
echo "#define HAVE_ERRNO_H 1" >>$@
-   echo "#define HAVE_GCVT 1" >>$@
echo "#define HAVE_SYSEXITS_H 1" >>$@
echo "#define HAVE_DLFCN_H 1" >>$@
echo "#define C_STACK_GROWS_DOWNWARD 1" >>$@
diff --git a/Makefile.haiku b/Makefile.haiku
index c6e33ab..63da7c0 100644
--- a/Makefile.haiku
+++ b/Makefile.haiku
@@ -91,7 +91,6 @@ chicken-config.h: chicken-defaults.h
echo "#define HAVE_ALLOCA_H 1" >>$@
echo "#define HAVE_GRP_H 1" >>$@
echo "#define HAVE_ERRNO_H 1" >>$@
-   echo "#define HAVE_GCVT 1" >>$@
echo "#define HAVE_MEMMOVE 1" >>$@
echo "#define C_STACK_GROWS_DOWNWARD 1" >>$@
echo "#define SIGIO 0" >>$@
diff --git a/Makefile.hurd b/Makefile.hurd
index 74c8c00..1a64158 100644
--- a/Makefile.hurd
+++ b/Makefile.hurd
@@ -92,7 +92,6 @@ chicken-config.h: chicken-defaults.h
echo "#define