Bug#220403: [patch] problem with floats (v2)

Nirgal Vourgère <[email protected]> Sun, 6 Feb 2011 21:29:46 +0000
Newsgroups gmane.linux.debian.devel.bugs.general,gmane.comp.db.mdb-tools.devel
Message-ID <[email protected]>
Looking at the file.c, I noticed we assume access floats are always 4 bytes, and double always 8. So it's not relevant to test what kind of float the local machine uses.
I think we can hard code the precision:
float: 24 bits mantice -> 8 characters
double: 53 bits mantice -> 16 characters
Keeping the same idea than bellow, attached is a revised and simplified patch. No more trouble with libm also.


On Sunday 06 February 2011 17:47:21 Nirgal Vourgère wrote:
> Hello
> 
> I have a problem introduced by the fix of http://bugs.debian.org/220403
> 
> Here was a part of the patch:
> Index: mdbtools-0.6pre1/src/libmdb/data.c
> ===================================================================
> --- mdbtools-0.6pre1.orig/src/libmdb/data.c
> +++ mdbtools-0.6pre1/src/libmdb/data.c
> @@ -857,7 +857,7 @@
>         break;
>         case MDB_FLOAT:
>             tf = mdb_get_single(buf, start);
> -           text = g_strdup_printf("%.*e",
> +           text = g_strdup_printf("%.*f",
>                 FLT_DIG - floor_log10(tf,1) - 1, tf);
>         break;
>         case MDB_DOUBLE:
> 
> 
> Rebuilding with MDB_DEBUG, I have this kind of output for prdata:
> 055b  00 24 64 46 
> column 1 is 1.5e+04
> 
> Python tells me the output should be
> struct.unpack('f', '\x00\x24\x64\x46')
> 14601.0
> 
> When reverting the patch, I get from prdata:
> 055b  00 24 64 46
> column 1 is 14601.0
> 
> I tried 1e-7 that was supposed not to work, and was the reason the patch was introduced. I have no problem:
> column 1 is 0.000000100000
> 
> 
> 
> You'll notice the unusual syntax "%.*f" that takes 2 arguments: the precision then the value.
> 
> I don't feel confortable exporting floats such as 1e-37 as a very long string, so I suggest we keep the 'e' format.
> 
> The precision however is wrong.
> I was about to use FLT_DIG, but it is repported buggy. :/
> Here, I have FLT_MANT_DIG==24 bits, meaning the maximum number without the exponent is 2^24=16777216
> The number of digits in a decimal representation is ceil(log10(16777216))=8
> The number of characters before the dot is not taken into account when specifying the precision
> printf("%.*e\n", 3, 1.23456789) -> 1.235e+00
> So that leave us with an extra number character. I assume this is usefull going arround roundings.
> 
> Does that sound ok?
> 
> Attached is the patch.
> I added some '-lm' for the linker to find the builtin math functions. I'm not very familiar with automake, you'd better check I did it right.
> 
> 
> Or maybe the original patch should just be reverted (for both MDB_FLOAT and MDB_DOUBLE)...
floats2.diff (text/x-patch, 1009 B)
Index: mdbtools-0.6pre1/src/libmdb/data.c
===================================================================
--- mdbtools-0.6pre1.orig/src/libmdb/data.c
+++ mdbtools-0.6pre1/src/libmdb/data.c
@@ -19,7 +19,6 @@
 
 #include "mdbtools.h"
 #include "time.h"
-#include "math.h"
 
 #ifdef DMALLOC
 #include "dmalloc.h"
@@ -803,6 +802,7 @@
 	return text;
 }
 
+#if 0
 int floor_log10(double f, int is_single)
 {
 	unsigned int i;
@@ -831,6 +831,7 @@
 		return (int)i;
 	}
 }
+#endif
 
 char *mdb_col_to_string(MdbHandle *mdb, void *buf, int start, int datatype, int size)
 {
@@ -856,13 +857,11 @@
 		break;
 		case MDB_FLOAT:
 			tf = mdb_get_single(buf, start);
-			text = g_strdup_printf("%.*e",
-				FLT_DIG - floor_log10(tf,1) - 1, tf);
+			text = g_strdup_printf("%.8e", tf);
 		break;
 		case MDB_DOUBLE:
 			td = mdb_get_double(buf, start);
-			text = g_strdup_printf("%.*e",
-				DBL_DIG - floor_log10(td,0) - 1, td);
+			text = g_strdup_printf("%.16e", td);
 		break;
 		case MDB_BINARY:
 		case MDB_TEXT: