[GRASS-dev] portable.h elimination

I've modified lib/vector/diglib to determine the byte ordering at run
time, rather than generating the portable.h header file.

The supplied test case passes. It might be useful to test it using dig
files generated on an architecture with the opposite byte order (I
have no way to test this). It might also be worth testing it on MinGW.

--
Glynn Clements <glynn@gclements.plus.com>

On Mon, March 5, 2007 14:06, Glynn Clements wrote:

I've modified lib/vector/diglib to determine the byte ordering at run
time, rather than generating the portable.h header file.

The supplied test case passes. It might be useful to test it using dig
files generated on an architecture with the opposite byte order (I
have no way to test this). It might also be worth testing it on MinGW.

How should I test ?

Moritz

On Tue, 13 Mar 2007, Moritz Lennert wrote:

On Mon, March 5, 2007 14:06, Glynn Clements wrote:

I've modified lib/vector/diglib to determine the byte ordering at run
time, rather than generating the portable.h header file.

The supplied test case passes. It might be useful to test it using dig
files generated on an architecture with the opposite byte order (I
have no way to test this). It might also be worth testing it on MinGW.

How should I test ?

Thanks for reminding me about this Moritz - I'd looked at it both quite a while ago and more recently after Glynn posted about the portable.h changes. I don't think there's any change on MinGW - the portable test in the lib/vector/diglib Makefile always failed anyway - presumably that's why it was conditionalised on not running on MinGW. If it is forced to run anyway, I get the following output:

==============TEST=============
gcc -L/c/grass/grass6/dist.i686-pc-mingw32/lib -Wl,--export-dynamic,--enable-runtime-pseudo-reloc -L/c/grass/forgrass/lib -I/c/grass/forgrass/include -g -O2 -I/c/grass/forgrass/include -I/c/grass/forgrass/include -I/c/grass/grass6/dist.i686-pc-mingw32/include -o OBJ.i686-pc-mingw32/test test.c -lgrass_dig2 -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_rtree -lgrass_rtree \
       -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_datetime -lxdr -liberty -lws2_32 -lz
cd OBJ.i686-pc-mingw32; PATH=".:/bin:/mingw/bin:/mingw/lib:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/jed09916/bin:/c/grass/grass6/dist.i686-pc-mingw32/lib" ./test; diff ./test.tmp ../test.ok
ERROR in read/write portable double, byte_order = 0
   Written: -1.3332999999999999e+0003E
   Read : -1.7976931348623157e+3083E
ERROR in read/write portable double, byte_order = 0
   Written: -2.2250738585072014e-3083E
   Read : -1.7976931348623157e+3083E
ERROR in read/write portable double, byte_order = 0
   Written: 0.0000000000000000e+0003E
   Read : -1.7976931348623157e+3083E
ERROR in read/write portable double, byte_order = 0
   Written: 2.2250738585072014e-3083E
   Read : -1.7976931348623157e+3083E
ERROR in read/write portable double, byte_order = 0
   Written: 1.3332999999999999e+0003E
   Read : -1.7976931348623157e+3083E
ERROR in read/write portable double, byte_order = 0
   Written: 1.7976931348623157e+3083E
   Read : -1.7976931348623157e+3083E
ERROR in read/write portable float, byte_order = 0
   Written: -3.40282347e+0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 0
   Written: -1.33329999e+0003E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 0
   Written: -1.17549435e-0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 0
   Written: 0.00000000e+0003E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 0
   Written: 1.17549435e-0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 0
   Written: 1.33329999e+0003E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 0
   Written: 3.40282347e+0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable long, byte_order = 0
   Written: -2147483647
   Read : -1048577
ERROR in read/write portable long, byte_order = 0
   Written: -123456789
   Read : -1048577
ERROR in read/write portable long, byte_order = 0
   Written: 0
   Read : -1048577
ERROR in read/write portable long, byte_order = 0
   Written: 123456789
   Read : -1048577
ERROR in read/write portable long, byte_order = 0
   Written: 2147483647
   Read : -1048577
ERROR in read/write portable int, byte_order = 0
   Written: -2147483647
   Read : -1048577
ERROR in read/write portable int, byte_order = 0
   Written: -123456789
   Read : -1048577
ERROR in read/write portable int, byte_order = 0
   Written: 0
   Read : -1048577
ERROR in read/write portable int, byte_order = 0
   Written: 123456789
   Read : -1048577
ERROR in read/write portable int, byte_order = 0
   Written: 2147483647
   Read : -1048577
ERROR in read/write portable short, byte_order = 0
   Written: -32768
   Read : -17
ERROR in read/write portable short, byte_order = 0
   Written: -12345
   Read : -17
ERROR in read/write portable short, byte_order = 0
   Written: 0
   Read : -17
ERROR in read/write portable short, byte_order = 0
   Written: 12345
   Read : -17
ERROR in read/write portable short, byte_order = 0
   Written: 32767
   Read : -17
ERROR in read/write portable char, byte_order = 0
   Written: -128
   Read : -1
ERROR in read/write portable char, byte_order = 0
   Written: -123
   Read : -1
ERROR in read/write portable char, byte_order = 0
   Written: 0
   Read : -1
ERROR in read/write portable char, byte_order = 0
   Written: 123
   Read : -1
ERROR in read/write portable char, byte_order = 0
   Written: 127
   Read : -1
ERROR in read/write portable double, byte_order = 1
   Written: -1.7976931348623157e+3083E
   Read : -1.#QNAN00000000000e+0003E
ERROR in read/write portable double, byte_order = 1
   Written: -1.3332999999999999e+0003E
   Read : -1.#QNAN00000000000e+0003E
ERROR in read/write portable double, byte_order = 1
   Written: -2.2250738585072014e-3083E
   Read : -1.#QNAN00000000000e+0003E
ERROR in read/write portable double, byte_order = 1
   Written: 0.0000000000000000e+0003E
   Read : -1.#QNAN00000000000e+0003E
ERROR in read/write portable double, byte_order = 1
   Written: 2.2250738585072014e-3083E
   Read : -1.#QNAN00000000000e+0003E
ERROR in read/write portable double, byte_order = 1
   Written: 1.3332999999999999e+0003E
   Read : -1.#QNAN00000000000e+0003E
ERROR in read/write portable double, byte_order = 1
   Written: 1.7976931348623157e+3083E
   Read : -1.#QNAN00000000000e+0003E
ERROR in read/write portable float, byte_order = 1
   Written: -3.40282347e+0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 1
   Written: -1.33329999e+0003E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 1
   Written: -1.17549435e-0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 1
   Written: 0.00000000e+0003E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 1
   Written: 1.17549435e-0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 1
   Written: 1.33329999e+0003E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable float, byte_order = 1
   Written: 3.40282347e+0383E
   Read : -1.#QNAN000e+0003E
ERROR in read/write portable long, byte_order = 1
   Written: -2147483647
   Read : -4097
ERROR in read/write portable long, byte_order = 1
   Written: -123456789
   Read : -4097
ERROR in read/write portable long, byte_order = 1
   Written: 0
   Read : -4097
ERROR in read/write portable long, byte_order = 1
   Written: 123456789
   Read : -4097
ERROR in read/write portable long, byte_order = 1
   Written: 2147483647
   Read : -4097
ERROR in read/write portable int, byte_order = 1
   Written: -2147483647
   Read : -4097
ERROR in read/write portable int, byte_order = 1
   Written: -123456789
   Read : -4097
ERROR in read/write portable int, byte_order = 1
   Written: 0
   Read : -4097
ERROR in read/write portable int, byte_order = 1
   Written: 123456789
   Read : -4097
ERROR in read/write portable int, byte_order = 1
   Written: 2147483647
   Read : -4097
ERROR in read/write portable short, byte_order = 1
   Written: -32768
   Read : -4097
ERROR in read/write portable short, byte_order = 1
   Written: -12345
   Read : -4097
ERROR in read/write portable short, byte_order = 1
   Written: 0
   Read : -4097
ERROR in read/write portable short, byte_order = 1
   Written: 12345
   Read : -4097
ERROR in read/write portable short, byte_order = 1
   Written: 32767
   Read : -4097
ERROR in read/write portable char, byte_order = 1
   Written: -128
   Read : -1
ERROR in read/write portable char, byte_order = 1
   Written: -123
   Read : -1
ERROR in read/write portable char, byte_order = 1
   Written: 0
   Read : -1
ERROR in read/write portable char, byte_order = 1
   Written: 123
   Read : -1
ERROR in read/write portable char, byte_order = 1
   Written: 127
   Read : -1
Binary files ./test.tmp and ../test.ok differ
make: *** [OBJ.i686-pc-mingw32/test] Error 1

For some reason (can't quite remember why) I changed test.c in the past to try and get this to work. ISTR someone suggested somewhere extra seeks were needed on Windows. This is what I changed:

--- test.c.orig Tue Mar 6 12:05:41 2007
+++ test.c.pk Fri Dec 15 10:55:33 2006
@@ -62,6 +62,7 @@
          for (j=0; j < 7; j++)
            {
        fprintf (fp.file, "double ");
+ dig_fseek (&fp, 0, SEEK_CUR);
              dig__fwrite_port_D ( &(td[j]), 1, &fp);
              dig_fseek (&fp, -(PORT_DOUBLE), SEEK_CUR);
        dig__fread_port_D (&db, 1, &fp);
@@ -76,6 +77,7 @@
          for (j=0; j < 7; j++)
            {
        fprintf (fp.file, "float ");
+ dig_fseek (&fp, 0, SEEK_CUR);
        dig__fwrite_port_F ( &(tf[j]), 1, &fp);
              dig_fseek (&fp, -(PORT_FLOAT), SEEK_CUR);
        dig__fread_port_F (&fb, 1, &fp);
@@ -91,6 +93,7 @@
          for (j=0; j < 5; j++)
            {
        fprintf (fp.file, "long ");
+ dig_fseek (&fp, 0, SEEK_CUR);
              dig__fwrite_port_L ( &(tl[j]), 1, &fp);
              dig_fseek (&fp, -(PORT_LONG), SEEK_CUR);
        dig__fread_port_L (&lb, 1, &fp);
@@ -106,6 +109,7 @@
          for (j=0; j < 5; j++)
            {
        fprintf (fp.file, "int ");
+ dig_fseek (&fp, 0, SEEK_CUR);
              dig__fwrite_port_I ( &(ti[j]), 1, &fp);
              dig_fseek (&fp, -(PORT_INT), SEEK_CUR);
        dig__fread_port_I (&ib, 1, &fp);
@@ -121,6 +125,7 @@
          for (j=0; j < 5; j++)
            {
        fprintf (fp.file, "short ");
+ dig_fseek (&fp, 0, SEEK_CUR);
              dig__fwrite_port_S ( &(ts[j]), 1, &fp);
              dig_fseek (&fp, -(PORT_SHORT), SEEK_CUR);
        dig__fread_port_S (&sb, 1, &fp);
@@ -135,6 +140,7 @@
          for (j=0; j < 5; j++)
            {
        fprintf (fp.file, "char ");
+ dig_fseek (&fp, 0, SEEK_CUR);
              dig__fwrite_port_C ( &(tc[j]), 1, &fp);
              dig_fseek (&fp, -(PORT_CHAR), SEEK_CUR);
        dig__fread_port_C (&cb, 1, &fp);

And with that the output from the test is now:

==============TEST=============
gcc -L/c/grass/grass6/dist.i686-pc-mingw32/lib -Wl,--export-dynamic,--enable-runtime-pseudo-reloc -L/c/grass/forgrass/lib -I/c/grass/forgrass/include -g -O2 -I/c/grass/forgrass/include -I/c/grass/forgrass/include -I/c/grass/grass6/dist.i686-pc-mingw32/include -o OBJ.i686-pc-mingw32/test test.c -lgrass_dig2 -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_rtree -lgrass_rtree \
       -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_datetime -lxdr -liberty -lws2_32 -lz
cd OBJ.i686-pc-mingw32; PATH=".:/bin:/mingw/bin:/mingw/lib:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/jed09916/bin:/c/grass/grass6/dist.i686-pc-mingw32/lib" ./test; diff ./test.tmp ../test.ok
Binary files ./test.tmp and ../test.ok differ
make: *** [OBJ.i686-pc-mingw32/test] Error 1

i.e. the test files are still different, but it doesn't have lots of errors while running. I have no idea if/how that is significant!

Paul

Paul Kelly wrote:

>> I've modified lib/vector/diglib to determine the byte ordering at run
>> time, rather than generating the portable.h header file.
>>
>> The supplied test case passes. It might be useful to test it using dig
>> files generated on an architecture with the opposite byte order (I
>> have no way to test this). It might also be worth testing it on MinGW.
>
> How should I test ?

Thanks for reminding me about this Moritz - I'd looked at it both quite a
while ago and more recently after Glynn posted about the portable.h
changes. I don't think there's any change on MinGW - the portable test in
the lib/vector/diglib Makefile always failed anyway - presumably that's
why it was conditionalised on not running on MinGW. If it is forced to run
anyway, I get the following output:

For some reason (can't quite remember why) I changed test.c in the past to
try and get this to work. ISTR someone suggested somewhere extra seeks
were needed on Windows.

You need to seek when switching between reading and writing. Windows'
FILE structure only has a single buffer (rather than separate
read/write buffers), so you need to force it to flush/discard any
buffered data.

This is what I changed:

Can you commit those, please.

And with that the output from the test is now:

==============TEST=============
gcc -L/c/grass/grass6/dist.i686-pc-mingw32/lib -Wl,--export-dynamic,--enable-runtime-pseudo-reloc -L/c/grass/forgrass/lib -I/c/grass/forgrass/include -g -O2 -I/c/grass/forgrass/include -I/c/grass/forgrass/include -I/c/grass/grass6/dist.i686-pc-mingw32/include -o OBJ.i686-pc-mingw32/test test.c -lgrass_dig2 -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_rtree -lgrass_rtree \
       -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_datetime -lxdr -liberty -lws2_32 -lz
cd OBJ.i686-pc-mingw32; PATH=".:/bin:/mingw/bin:/mingw/lib:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/jed09916/bin:/c/grass/grass6/dist.i686-pc-mingw32/lib" ./test; diff ./test.tmp ../test.ok
Binary files ./test.tmp and ../test.ok differ
make: *** [OBJ.i686-pc-mingw32/test] Error 1

i.e. the test files are still different, but it doesn't have lots of
errors while running. I have no idea if/how that is significant!

The stream of errors indicates that what was read back doesn't match
what was written, i.e. a problem with low-level I/O.

The "diff" failure indicates that what was written wasn't what was
expected. This could be a text/binary issue (I don't see $(FMODE_OBJ)
in the link command); it could even be due to differences in
floating-point rounding (even if only the lowest bit of a double is
wrong, the test will still fail).

If adding $(FMODE_OBJ) to the test doesn't work, convert test.tmp and
test.ok to hex (with "od -t x ...") and compare the hex dumps.

--
Glynn Clements <glynn@gclements.plus.com>

Moritz Lennert wrote:

> I've modified lib/vector/diglib to determine the byte ordering at run
> time, rather than generating the portable.h header file.
>
> The supplied test case passes. It might be useful to test it using dig
> files generated on an architecture with the opposite byte order (I
> have no way to test this). It might also be worth testing it on MinGW.

How should I test ?

Try using the vector commands on existing vector maps, and creating
some new vector maps.

--
Glynn Clements <glynn@gclements.plus.com>

On Wed, 14 Mar 2007, Glynn Clements wrote:

Paul Kelly wrote:

For some reason (can't quite remember why) I changed test.c in the past to
try and get this to work. ISTR someone suggested somewhere extra seeks
were needed on Windows.

You need to seek when switching between reading and writing. Windows'
FILE structure only has a single buffer (rather than separate
read/write buffers), so you need to force it to flush/discard any
buffered data.

Do you think extra seeks need added anywhere else in the library, or is it just this test program that is affected?

This is what I changed:

Can you commit those, please.

And with that the output from the test is now:

==============TEST=============
gcc -L/c/grass/grass6/dist.i686-pc-mingw32/lib -Wl,--export-dynamic,--enable-runtime-pseudo-reloc -L/c/grass/forgrass/lib -I/c/grass/forgrass/include -g -O2 -I/c/grass/forgrass/include -I/c/grass/forgrass/include -I/c/grass/grass6/dist.i686-pc-mingw32/include -o OBJ.i686-pc-mingw32/test test.c -lgrass_dig2 -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_rtree -lgrass_rtree \
       -lgrass_gis -lgrass_datetime -lxdr -liberty -lws2_32 -lz -lgrass_datetime -lxdr -liberty -lws2_32 -lz
cd OBJ.i686-pc-mingw32; PATH=".:/bin:/mingw/bin:/mingw/lib:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/grass/forgrass/bin:/c/grass/forgrass/lib:/c/tcl/bin:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem:/c/Program Files/QuickTime/QTSystem/:/c/jed09916/bin:/c/grass/grass6/dist.i686-pc-mingw32/lib" ./test; diff ./test.tmp ../test.ok
Binary files ./test.tmp and ../test.ok differ
make: *** [OBJ.i686-pc-mingw32/test] Error 1

i.e. the test files are still different, but it doesn't have lots of
errors while running. I have no idea if/how that is significant!

The stream of errors indicates that what was read back doesn't match
what was written, i.e. a problem with low-level I/O.

The "diff" failure indicates that what was written wasn't what was
expected. This could be a text/binary issue (I don't see $(FMODE_OBJ)
in the link command); it could even be due to differences in
floating-point rounding (even if only the lowest bit of a double is
wrong, the test will still fail).

None of those - it was just that I had put the dig_fseek() call in too late - it should have been before the call to fprintf(). With it before fprintf(), now everything works even without bothering about $(FMODE_OBJ) so I have committed the changes to CVS and enabled the test for MinGW in the Makefile.

Paul

Paul Kelly wrote:

>> For some reason (can't quite remember why) I changed test.c in the past to
>> try and get this to work. ISTR someone suggested somewhere extra seeks
>> were needed on Windows.
>
> You need to seek when switching between reading and writing. Windows'
> FILE structure only has a single buffer (rather than separate
> read/write buffers), so you need to force it to flush/discard any
> buffered data.

Do you think extra seeks need added anywhere else in the library, or is it
just this test program that is affected?

I would think that most uses of the diglib I/O use either read-only or
write-only streams, so shouldn't be affected. And programs which do
both will normally follow a pattern of seeking followed by a sequence
of reads or a sequence of writes.

Writing something then reading whatever happens to follow what you've
just written seems unlikely. Reading then writing seems more likely.
Also, this isn't specific to the diglib I/O, but anything using ANSI
stdio.

> The "diff" failure indicates that what was written wasn't what was
> expected. This could be a text/binary issue (I don't see $(FMODE_OBJ)
> in the link command); it could even be due to differences in
> floating-point rounding (even if only the lowest bit of a double is
> wrong, the test will still fail).

None of those - it was just that I had put the dig_fseek() call in too
late - it should have been before the call to fprintf(). With it before
fprintf(), now everything works even without bothering about $(FMODE_OBJ)
so I have committed the changes to CVS and enabled the test for MinGW in
the Makefile.

I've noticed that test.ok doesn't include any '\n' characters, so EOL
conversions won't affect the test case. But, in general, every program
needs to link against $(FMODE_OBJ). Those which use the cmd/inter/etc
targets from Module.make are automatically covered, but anything with
an explicit linking command needs to handle this manually.

It might be worth adding a make function for linking executables to
e.g. Rules.make, so that we don't have to deal with this issue (and
similar issues) in dozens of individual Makefiles. There are quite a
lot of programs which aren't built with the standard cmd/inter/etc
targets.

--
Glynn Clements <glynn@gclements.plus.com>