Opened 8 years ago

Closed 7 years ago

#1866 closed defect (fixed)

broken db driver communication in winGRASS 7

Reported by: martinl Owned by: grass-dev@…
Priority: normal Milestone: 7.0.0
Component: Database Version: unspecified
Keywords: sqlite, wingrass Cc:
CPU: Unspecified Platform: MSWindows 2K

Description

In winGRASS 7 also SQLite driver is not working (see #1844 from DBF driver). NC (basic) dataset.

db.describe census

doesn't report any error, it just hangs. Adding some extra debug messages, I found the line when it stops. It's source:grass/trunk/lib/db/dbmi_base/xdrcolumn.c#L56. First three columns are retrieved (cat, OBJRCTID, AREA). It stops on fourth column and waiting for response.

Any idea where could be a problem?

This bugs affects only GRASS 7, SQLite driver in GRASS 6 seems to work.

Attachments (2)

hexdump.txt (12.0 KB) - added by mmetz 7 years ago.
hexdump for v.info -c map=boundary_county
db_describe.diff (1.2 KB) - added by mlennert 7 years ago.
patch for better error message and more explanation in man page

Download all attachments as: .zip

Change History (54)

comment:1 Changed 8 years ago by martinl

Update: string for name of fourth column has invalid size (should be for this column - PERIMETER - 10). Seems like buffer overflow or so(?)

comment:2 Changed 8 years ago by martinl

Wild guess: could be related to invalid call of free() (instead of dbfree()). Any tips how to determine such invalid usage?

comment:3 in reply to:  2 ; Changed 8 years ago by mmetz

Replying to martinl:

Wild guess: could be related to invalid call of free() (instead of dbfree()). Any tips how to determine such invalid usage?

I have changed the default error reporting setting for db drivers in r54828. Maybe something useful is now printed. Please test.

Markus M

comment:4 in reply to:  3 ; Changed 8 years ago by martinl

Replying to mmetz:

I have changed the default error reporting setting for db drivers in r54828. Maybe something useful is now printed. Please test.

thanks, anyway unfortunately nothing is printed.

comment:5 Changed 8 years ago by martinl

To be more specific, when adding some extra debug messages

Index: lib/db/dbmi_base/xdrcolumn.c
===================================================================
--- dbmi_base/xdrcolumn.c       (revision 54829)
+++ dbmi_base/xdrcolumn.c       (working copy)
@@ -53,6 +53,8 @@
 int db__recv_column_definition(dbColumn * column)
 {
     DB_RECV_STRING(&column->columnName);
+    G_debug(0, "col: %s (%d)", db_get_string(&column->columnName),
+           db_sizeof_string(&column->columnName));
     DB_RECV_STRING(&column->description);
     DB_RECV_INT(&column->sqlDataType);
     DB_RECV_INT(&column->hostDataType);

I am getting

db.describe census
D0/0: col: cat (4)
D0/0: col: OBJECTID (9)
D0/0: col: AREA (5)
D0/0: col:  (2573)

comment:6 in reply to:  4 ; Changed 8 years ago by mmetz

Replying to martinl:

Replying to mmetz:

I have changed the default error reporting setting for db drivers in r54828. Maybe something useful is now printed. Please test.

thanks, anyway unfortunately nothing is printed.

I get exactly the same behaviour with the dbf driver: nothing happens, no error message and the driver hangs.

Markus M

comment:7 in reply to:  6 ; Changed 8 years ago by martinl

Replying to mmetz:

I get exactly the same behaviour with the dbf driver: nothing happens, no error message and the driver hangs.

strangely, in GRASS 6 both DBF and SQLite seems to work.

comment:8 in reply to:  7 ; Changed 8 years ago by mmetz

Replying to martinl:

Replying to mmetz:

I get exactly the same behaviour with the dbf driver: nothing happens, no error message and the driver hangs.

strangely, in GRASS 6 both DBF and SQLite seems to work.

Strangely, in GRASS 7 both DBF and SQLite driver seem to work. What is broken is the pipe for sending data back and forth between the module and the driver.

Markus M

comment:9 in reply to:  8 ; Changed 8 years ago by mmetz

Replying to mmetz:

Replying to martinl:

Replying to mmetz:

I get exactly the same behaviour with the dbf driver: nothing happens, no error message and the driver hangs.

strangely, in GRASS 6 both DBF and SQLite seems to work.

Strangely, in GRASS 7 both DBF and SQLite driver seem to work. What is broken is the pipe for sending data back and forth between the module and the driver.

I added some more debug info and get for db.describe boundary_county

D0/0: send column name: cat
D0/0: D0/0: receive string of len: 4
D0/0: string alloc: 0
D0/0: string received: cat
D0/0: column name: cat
D0/0: receive string of len: 1
D0/0: string alloc: 0
D0/0: string received:
send column name: AREA
D0/0: D0/0: receive string of len: 5
D0/0: string alloc: 0
D0/0: string received: AREA
D0/0: column name: AREA
D0/0: receive string of len: 1
D0/0: string alloc: 0
D0/0: string received:
send column name: PERIMETER
D0/0: D0/0: receive string of len: 2573
D0/0: string alloc: 0
send column name: FIPS
D0/0: send column name: NAME
D0/0: send column name: NAME_LOCAS
D0/0: send column name: DOT_DISTRI
D0/0: send column name: DOT_DIVISI
D0/0: send column name: DOT_COUNTY
D0/0: send column name: COUNTY_100
D0/0: send column name: DOT_GROUP_
D0/0: send column name: ACRES
D0/0: send column name: ABBR_5CHAR
D0/0: send column name: ABBR_4CHAR
D0/0: send column name: ABBR_2CHAR
D0/0: send column name: Z_MEAN
D0/0: send column name: Z_MIN
D0/0: send column name: Z_MAX
D0/0: send column name: Z_ZONE
D0/0: send column name: CO_CENSUS
D0/0: send column name: DIV_CONTAC
D0/0: send column name: DIST_CONTA
D0/0: send column name: CO_WIKIPED
D0/0: send column name: Shape_Leng
D0/0: send column name: Shape_Area

Apparently the receiving end of the pipe [0] is broken. Anybody has any idea how this could happen? There are no obvious differences in the code base between 6.4. and 7.0.

Markus M

[0] https://trac.osgeo.org/grass/browser/grass/trunk/lib/db/dbmi_client/start.c#L130

comment:10 in reply to:  9 Changed 7 years ago by martinl

Replying to mmetz:

Apparently the receiving end of the pipe [0] is broken. Anybody has any idea how this could happen? There are no obvious differences in the code base between 6.4. and 7.0.

Any idea here? This is really critical issue. Currently winGRASS 7 is completely broken when working with vector data. Two main DB drivers (SQLite, DBF) are broken.

comment:11 Changed 7 years ago by cmbarton

This seems to be working fine on the Mac for R55039. I just did db.describe for forestations in the nc_spm_08 demo set for data in sqlite. No problem.

Michael

comment:12 in reply to:  11 ; Changed 7 years ago by mmetz

Summary: broken SQLite driver in winGRASS 7broken db driver communication in winGRASS 7

Replying to cmbarton:

This seems to be working fine on the Mac for R55039. I just did db.describe for forestations in the nc_spm_08 demo set for data in sqlite. No problem.

The drivers are working just fine. Someone is not listening properly, i.e. the dblib as used by modules, because the drivers are sending correct information, but the dblib receives bargage. There are some parts in the code, in the db lib, in gislib for spawning and in the wxGUI that have switches for Windows. But the relevant code in the db drivers, the db lib and gislib is identical between 6.4, 6.5 and 7. The only differences I could find between trunk and devbr6/relbr64 are in the GRASS startup script and in the wxGUI. Is it possible that the wxGUI or the startup script are highjacking file descriptors of the pipes?

Markus M

comment:13 in reply to:  11 Changed 7 years ago by martinl

Replying to cmbarton:

This seems to be working fine on the Mac for R55039. I just did db.describe for forestations in the nc_spm_08 demo set for data in sqlite. No problem.

Thanks for testing. It's working also on GNU/Linux too, it's problem only on Windows.

comment:14 in reply to:  12 ; Changed 7 years ago by mmetz

Replying to mmetz:

Replying to cmbarton:

This seems to be working fine on the Mac for R55039. I just did db.describe for forestations in the nc_spm_08 demo set for data in sqlite. No problem.

The drivers are working just fine. Someone is not listening properly, i.e. the dblib as used by modules, because the drivers are sending correct information, but the dblib receives bargage. There are some parts in the code, in the db lib, in gislib for spawning and in the wxGUI that have switches for Windows. But the relevant code in the db drivers, the db lib and gislib is identical between 6.4, 6.5 and 7. The only differences I could find between trunk and devbr6/relbr64 are in the GRASS startup script and in the wxGUI. Is it possible that the wxGUI or the startup script are highjacking file descriptors of the pipes?

The db driver communication was broken with r53257, September 2012. For some unknown reason, wingrass needs to be linked against -lxdr, even though all references to the xdr lib should have been eliminated. Fixed in r55332, but this ticket should not be closed until the ultimate reason is known.

Markus M

comment:15 in reply to:  14 ; Changed 7 years ago by glynn

Replying to mmetz:

Fixed in r55332

r55332 doesn't fix anything, but instead breaks linking (because libxdr typically won't exist). Reverted in r55335.

In what way does the DBMI client lib receive "garbage"? Is the data simply corrupted, or does it bear absolutely no relation to what the driver sends?

comment:16 Changed 7 years ago by martinl

seems to be related to #1796, #1844, and probably more tickets. It's very good news, that we know at least what is source of the problems (thanks MarkusM for discovering that).

comment:17 in reply to:  15 ; Changed 7 years ago by mmetz

Replying to glynn:

Replying to mmetz:

Fixed in r55332

r55332 doesn't fix anything, but instead breaks linking (because libxdr typically won't exist).

libxdr exists in osgeo4w [0]. I don't know why r55332 works, but it works. IOW, r53256 works, r53257 does not work.

Reverted in r55335.

I am inclined to revert r55335 until a proper solution is found.

In what way does the DBMI client lib receive "garbage"? Is the data simply corrupted, or does it bear absolutely no relation to what the driver sends?

The communication pipe first sends the size of the string, then the string itself [1]. After the first few transmissions, the size of the string as sent by the driver is correct, but the size of the string as received by the dblib is too large. That means the driver continues sending correct data with db__send_string() while db__recv_string() still waits for a very large string to appear, which never appears. As a result, nothing happens and the modules using dblib hang.

[0] http://download.osgeo.org/osgeo4w/release/libxdr/libxdr/ [1] https://trac.osgeo.org/grass/browser/grass/trunk/lib/db/dbmi_base/xdrstring.c#L86

comment:18 in reply to:  17 Changed 7 years ago by martinl

Replying to mmetz:

Reverted in r55335.

I am inclined to revert r55335 until a proper solution is found.

agreed, note that since sep 2012 db drivers like dbf and sqlite are broken on Windows. In other words, winGRASS7 completely unusable when working with attribute tables. This is really critical issue.

comment:19 in reply to:  17 ; Changed 7 years ago by mmetz

Replying to mmetz:

Replying to glynn:

Replying to mmetz:

Fixed in r55332

r55332 doesn't fix anything, but instead breaks linking (because libxdr typically won't exist).

libxdr exists in osgeo4w [0]. I don't know why r55332 works, but it works. IOW, r53256 works, r53257 does not work.

Reverted in r55335.

I am inclined to revert r55335 until a proper solution is found.

In what way does the DBMI client lib receive "garbage"? Is the data simply corrupted, or does it bear absolutely no relation to what the driver sends?

The communication pipe first sends the size of the string, then the string itself [1]. After the first few transmissions, the size of the string as sent by the driver is correct, but the size of the string as received by the dblib is too large. That means the driver continues sending correct data with db__send_string() while db__recv_string() still waits for a very large string to appear, which never appears. As a result, nothing happens and the modules using dblib hang.

[0] http://download.osgeo.org/osgeo4w/release/libxdr/libxdr/ [1] https://trac.osgeo.org/grass/browser/grass/trunk/lib/db/dbmi_base/xdrstring.c#L86

Some more information:

Linking only dbmi_base and/or dbmi_client against libxdr does not help. Linking only gislib against xdrlib does not help either. The change in r55332 is the only way I found to get the db drivers working.

Linking against xdr has been removed in order to get GRASS compiled on Android, which is not a supported platform. I prefer to have GRASS working on the supported platforms, even if it is a hack, and am thus restoring r55332 until the underlying cause of the bug is found and fixed.

Testing in the NC dataset with

v.info -c map=boundary_county

I get with additional debug info in wingrass7 without libxdr

Displaying column types/names for database
D0/0: send string of len: 38
D0/0: send string of len: 1
D0/0: receive string of len: 38
D0/0: string received: C:\GRASSdata\nc_spm
D0/0: receive string of len: 1
D0/0: string received:
D0/0: send string of len: 16
D0/0: receive string of len: 16
D0/0: string received: boundary_county
D0/0: send column name: cat
D0/0: send string of len: 4
D0/0: D0/0: receive string of len: 4

The pipe is now broken because it continues with

D0/0: send string of len: 1
D0/0: send column name: AREA
D0/0: send string of len: 5
D0/0: send string of len: 1
D0/0: send column name: PERIMETER
D0/0: send string of len: 10
D0/0: send string of len: 1
D0/0: string received: cat
D0/0: column name: cat
D0/0: receive string of len: 1
D0/0: string received:

Here garbage is received: the string length received is 1280 instead of 5:

D0/0: receive string of len: 1280
send column name: FIPS
D0/0: send string of len: 5
D0/0: send string of len: 1
D0/0: send column name: NAME
D0/0: send string of len: 5
D0/0: send string of len: 1
D0/0: send column name: NAME_LOCAS
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: DOT_DISTRI
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: DOT_DIVISI
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: DOT_COUNTY
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: COUNTY_100
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: DOT_GROUP_
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: ACRES
D0/0: send string of len: 6
D0/0: send string of len: 1
D0/0: send column name: ABBR_5CHAR
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: ABBR_4CHAR
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: ABBR_2CHAR
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: Z_MEAN
D0/0: send string of len: 7
D0/0: send string of len: 1
D0/0: send column name: Z_MIN
D0/0: send string of len: 6
D0/0: send string of len: 1
D0/0: send column name: Z_MAX
D0/0: send string of len: 6
D0/0: send string of len: 1
D0/0: send column name: Z_ZONE
D0/0: send string of len: 7
D0/0: send string of len: 1
D0/0: send column name: CO_CENSUS
D0/0: send string of len: 10
D0/0: send string of len: 1
D0/0: send column name: DIV_CONTAC
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: DIST_CONTA
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: CO_WIKIPED
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: Shape_Leng
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send column name: Shape_Area
D0/0: send string of len: 11
D0/0: send string of len: 1
D0/0: send string of len: 16
D0/0: send string of len: 1

Linking against xdrlib I get

Displaying column types/names for data
D0/0: send string of len: 38
D0/0: send string of len: 1
D0/0: receive string of len: 38
D0/0: string received: C:\GRASSdata\nc
D0/0: receive string of len: 1
D0/0: string received:
D0/0: send string of len: 16
D0/0: receive string of len: 16
D0/0: string received: boundary_county
D0/0: send column name: cat
D0/0: send string of len: 4
D0/0: D0/0: receive string of len: 4
D0/0: string received: cat
D0/0: column name: cat
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: AREA
D0/0: send string of len: 5
D0/0: D0/0: receive string of len: 5
D0/0: string received: AREA
D0/0: column name: AREA
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: PERIMETER
D0/0: send string of len: 10
D0/0: D0/0: receive string of len: 10
D0/0: string received: PERIMETER
D0/0: column name: PERIMETER
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: FIPS
D0/0: send string of len: 5
D0/0: D0/0: receive string of len: 5
D0/0: string received: FIPS
D0/0: column name: FIPS
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: NAME
D0/0: send string of len: 5
D0/0: D0/0: receive string of len: 5
D0/0: string received: NAME
D0/0: column name: NAME
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: NAME_LOCAS
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: NAME_LOCAS
D0/0: column name: NAME_LOCAS
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: DOT_DISTRI
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: DOT_DISTRI
D0/0: column name: DOT_DISTRI
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: DOT_DIVISI
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: DOT_DIVISI
D0/0: column name: DOT_DIVISI
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: DOT_COUNTY
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: DOT_COUNTY
D0/0: column name: DOT_COUNTY
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: COUNTY_100
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: COUNTY_100
D0/0: column name: COUNTY_100
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: DOT_GROUP_
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: DOT_GROUP_
D0/0: column name: DOT_GROUP_
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: ACRES
D0/0: send string of len: 6
D0/0: D0/0: receive string of len: 6
D0/0: string received: ACRES
D0/0: column name: ACRES
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: ABBR_5CHAR
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: ABBR_5CHAR
D0/0: column name: ABBR_5CHAR
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: ABBR_4CHAR
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: ABBR_4CHAR
D0/0: column name: ABBR_4CHAR
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: ABBR_2CHAR
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: ABBR_2CHAR
D0/0: column name: ABBR_2CHAR
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: Z_MEAN
D0/0: send string of len: 7
D0/0: D0/0: receive string of len: 7
D0/0: string received: Z_MEAN
D0/0: column name: Z_MEAN
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: Z_MIN
D0/0: send string of len: 6
D0/0: D0/0: receive string of len: 6
D0/0: string received: Z_MIN
D0/0: column name: Z_MIN
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: Z_MAX
D0/0: send string of len: 6
D0/0: D0/0: receive string of len: 6
D0/0: string received: Z_MAX
D0/0: column name: Z_MAX
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: Z_ZONE
D0/0: send string of len: 7
D0/0: D0/0: receive string of len: 7
D0/0: string received: Z_ZONE
D0/0: column name: Z_ZONE
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: CO_CENSUS
D0/0: send string of len: 10
D0/0: D0/0: receive string of len: 10
D0/0: string received: CO_CENSUS
D0/0: column name: CO_CENSUS
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: DIV_CONTAC
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: DIV_CONTAC
D0/0: column name: DIV_CONTAC
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: DIST_CONTA
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: DIST_CONTA
D0/0: column name: DIST_CONTA
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: CO_WIKIPED
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: CO_WIKIPED
D0/0: column name: CO_WIKIPED
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: Shape_Leng
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: Shape_Leng
D0/0: column name: Shape_Leng
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send column name: Shape_Area
D0/0: send string of len: 11
D0/0: D0/0: receive string of len: 11
D0/0: string received: Shape_Area
D0/0: column name: Shape_Area
send string of len: 1
D0/0: D0/0: receive string of len: 1
D0/0: string received:
send string of len: 16
D0/0: D0/0: receive string of len: 16
D0/0: string received: boundary_county
send string of len: 1
D0/0: receive string of len: 1
D0/0: string received:
INTEGER|cat
DOUBLE PRECISION|AREA
DOUBLE PRECISION|PERIMETER
DOUBLE PRECISION|FIPS
CHARACTER VARYING|NAME
CHARACTER VARYING|NAME_LOCAS
INTEGER|DOT_DISTRI
INTEGER|DOT_DIVISI
INTEGER|DOT_COUNTY
INTEGER|COUNTY_100
CHARACTER VARYING|DOT_GROUP_
DOUBLE PRECISION|ACRES
CHARACTER VARYING|ABBR_5CHAR
CHARACTER VARYING|ABBR_4CHAR
CHARACTER VARYING|ABBR_2CHAR
DOUBLE PRECISION|Z_MEAN
DOUBLE PRECISION|Z_MIN
DOUBLE PRECISION|Z_MAX
DOUBLE PRECISION|Z_ZONE
CHARACTER VARYING|CO_CENSUS
CHARACTER VARYING|DIV_CONTAC
CHARACTER VARYING|DIST_CONTA
CHARACTER VARYING|CO_WIKIPED
DOUBLE PRECISION|Shape_Leng
DOUBLE PRECISION|Shape_Area
GRASS 7.0.svn>

comment:20 in reply to:  17 ; Changed 7 years ago by glynn

Replying to mmetz:

Reverted in r55335.

I am inclined to revert r55335 until a proper solution is found.

Reinstating the hack makes it less likely that a proper solution will be found.

In what way does the DBMI client lib receive "garbage"? Is the data simply corrupted, or does it bear absolutely no relation to what the driver sends?

The communication pipe first sends the size of the string, then the string itself [1]. After the first few transmissions, the size of the string as sent by the driver is correct, but the size of the string as received by the dblib is too large.

That doesn't really help. What would help would be the actual bytes. But I can hazard a guess based upon comment:19:

Here garbage is received: the string length received is 1280 instead of 5:

5 = 0x00000005 = "\x05\x00\x00\x00"

1280 = 0x00000500 = "\x00\x05\x00\x00"

It appears that an extra character has been inserted somewhere in the stream. The usual suspect is LF->CRLF conversion. That is supposed to be prevented by linking everything against fmode.o, but it's possible that isn't happening for some reason, or something else is overriding it. Also, G_gisinit() and G__gisinit() explicitly set "_fmode = O_BINARY", but those functions don't appear to be called by DBMI drivers (although they typically call other libgis functions, e.g. G__setenv()).

But rather than guess, it would be better to instrument db__send() and db__recv() to dump the data as hex.

comment:21 in reply to:  19 Changed 7 years ago by glynn

Replying to mmetz:

Linking against xdr has been removed in order to get GRASS compiled on Android,

Linking against XDR has been removed because GRASS has its own XDR functions, and won't use the ones in libxdr (or libc) even if they're present.

As for why GRASS stopped using XDR, Android support was basically the last straw. I had already needed to port BSD-XDR to Cygwin (which stopped shipping it in 2001), and the same issue would probably apply to any future platform.

comment:22 in reply to:  20 ; Changed 7 years ago by mmetz

Replying to glynn:

Replying to mmetz:

Reverted in r55335.

I am inclined to revert r55335 until a proper solution is found.

Reinstating the hack makes it less likely that a proper solution will be found.

Hmm. For the last 6 months this problem has been simply ignored. Anybody trying to find a solution will have to modify to code anyway.

It appears that an extra character has been inserted somewhere in the stream. The usual suspect is LF->CRLF conversion. That is supposed to be prevented by linking everything against fmode.o, but it's possible that isn't happening for some reason, or something else is overriding it.

I get a compiler warning for fmode.c:

fmode.c:4:5: warning: '_fmode' redeclared without dllimport attribute: previous
dllimport ignored [-Wattributes]

Also, G_gisinit() and G__gisinit() explicitly set "_fmode = O_BINARY", but those functions don't appear to be called by DBMI drivers (although they typically call other libgis functions, e.g. G__setenv()).

Adding G_gisinit(), G_no_gisinit(), or "_fmode = O_BINARY", or any combination of these to main.c of the drivers does not help.

But rather than guess, it would be better to instrument db__send() and db__recv() to dump the data as hex.

Attached as hexdump.txt for v.info -c map=boundary_county. Please let me know if you want another format.

Changed 7 years ago by mmetz

Attachment: hexdump.txt added

hexdump for v.info -c map=boundary_county

comment:23 in reply to:  22 ; Changed 7 years ago by glynn

Replying to mmetz:

I get a compiler warning for fmode.c:

fmode.c:4:5: warning: '_fmode' redeclared without dllimport attribute: previous
dllimport ignored [-Wattributes]

FWIW, MinGW provides a /lib/binmode.o file for this purpose. Can you try:

make FMODE_OBJ=/lib/binmode.o

Adding G_gisinit(), G_no_gisinit(), or "_fmode = O_BINARY", or any combination of these to main.c of the drivers does not help.

If our fmode.o is wrong, it may actually prevent the run-time _fmode setting from working by introducing a new _fmode variable which shadows the real one.

But rather than guess, it would be better to instrument db__send() and db__recv() to dump the data as hex.

Attached as hexdump.txt for v.info -c map=boundary_county. Please let me know if you want another format.

Can you indicate whether the bytes are sent or received, and by which process? Also: where is this data coming from? db__send and db__recv? If so, the hexdump is even worse than the previous reports; e.g. the fifth entry shows something receiving 38 bytes but there is no record of it being sent.

comment:24 in reply to:  23 ; Changed 7 years ago by glynn

Replying to glynn:

FWIW, MinGW provides a /lib/binmode.o file for this purpose. Can you try:

make FMODE_OBJ=/lib/binmode.o

Another thing to try is:

#include <io.h>
...
_setmode(_fileno(stdin), _O_BINARY);
_setmode(_fileno(stdout), _O_BINARY);

comment:25 in reply to:  24 ; Changed 7 years ago by mmetz

Replying to glynn:

Replying to glynn:

FWIW, MinGW provides a /lib/binmode.o file for this purpose. Can you try:

make FMODE_OBJ=/lib/binmode.o

Not working because this has the effect of making binary-mode file translation the default, except that preopened files stdin, stderr, stdout remain text. The driver pipes are set to stdin and stdout in lib/db/dbmi_driver/driver.c.

Anyway, I have added

# if fmode.o causes trouble, try:
# FMODE_OBJ=/lib/binmode.o

to Grass.make in r55381 as a hint if problems with lib/gis/fmode.c arise. At the same time I have removed the -lxdr hack.

Another thing to try is:

#include <io.h>
...
_setmode(_fileno(stdin), _O_BINARY);
_setmode(_fileno(stdout), _O_BINARY);

I have added that to lib/db/dbmi_driver/driver.c and it works! Submitted in r55382. Do we need to add this globally, i.e. to gisinit()?

BTW, I think that lib/dbmi_driver/driver.c should be in lib/db/dbmi_client/ for consistency, or IOW there were lib/dbmi_client/start.c is. It seems that lib/dbmi_driver/ should only include db_d_* functions.

comment:26 in reply to:  25 ; Changed 7 years ago by glynn

Replying to mmetz:

Do we need to add this globally, i.e. to gisinit()?

I don't think so. stdin/stdout are normally text streams, so we probably want the CRLF<->LF conversions.

BTW, I think that lib/dbmi_driver/driver.c should be in lib/db/dbmi_client/ for consistency, or IOW there were lib/dbmi_client/start.c is. It seems that lib/dbmi_driver/ should only include db_d_* functions.

Drivers should be linked with -lgrass_dbmidriver and -lgrass_dbmibase. Clients should be linked with -lgrass_dbmiclient and -lgrass_dbmibase.

dbmi_driver should contain functions which are only of use to drivers, dbmi_client should contain functions which are only of use to clients, dbmi_base should contain functions which are of use to both.

db_driver() is the core of a driver's main() function, and belongs in dbmi_driver.

comment:27 in reply to:  26 ; Changed 7 years ago by mmetz

Replying to glynn:

Replying to mmetz:

Do we need to add this globally, i.e. to gisinit()?

I don't think so. stdin/stdout are normally text streams, so we probably want the CRLF<->LF conversions.

OK.

Should the fix be backported to GRASS 6? It seems that GRASS 6 is only working by accident on Windows because the osgeo4w XDR lib does what GRASS should do.

Markus M

comment:28 in reply to:  27 ; Changed 7 years ago by glynn

Replying to mmetz:

Should the fix be backported to GRASS 6?

Yes.

comment:29 in reply to:  28 ; Changed 7 years ago by mmetz

Replying to glynn:

Replying to mmetz:

Should the fix be backported to GRASS 6?

Yes.

Done in r55432,3.

comment:30 in reply to:  29 ; Changed 7 years ago by hellik

Replying to mmetz:

Replying to glynn:

Replying to mmetz:

Should the fix be backported to GRASS 6?

Yes.

Done in r55432,3.

maybe related?

Copy vector <firestations@PERMANENT> to current mapset as <myhell>
WARNING: Unable to start driver <dbf>
WARNING: Unable to copy table <myhell>
WARNING: Cannot copy <firestations@PERMANENT> to current mapset as <myhell>
v.db.addcol map=testvect@user1 columns=ti integer                               
ERROR: Unable to start driver <dbf>
ERROR: Cannot continue (problem adding column).

tested with

System Info                                                                     
GRASS version: 6.4.3svn                                                         
GRASS SVN Revision: 55442                                                       
GIS Library Revision: 50937 (2012-02-25)                                        
GDAL/OGR: 1.9.2                                                                 
PROJ4: Rel. 4.8.0, 6 March 2012                                                 
Python: 2.7.2                                                                   
wxPython: 2.8.12.1                                                              
Platform: Windows-7-6.1.7601-SP1 (OSGeo4W)     

Helmut

comment:31 in reply to:  30 ; Changed 7 years ago by mmetz

Replying to hellik:

Replying to mmetz:

Replying to glynn:

Replying to mmetz:

Should the fix be backported to GRASS 6?

Yes.

Done in r55432,3.

maybe related?

Copy vector <firestations@PERMANENT> to current mapset as <myhell>
WARNING: Unable to start driver <dbf>
WARNING: Unable to copy table <myhell>
WARNING: Cannot copy <firestations@PERMANENT> to current mapset as <myhell>

Should be fixed in r55473,4.

Markus M

comment:32 in reply to:  31 Changed 7 years ago by hellik

Replying to mmetz:

Should be fixed in r55473,4.

yes, thanks for the quick fix.

Helmut

comment:33 in reply to:  description ; Changed 7 years ago by neteler

Replying to martinl:

In winGRASS 7 also SQLite driver is not working (see #1844 from DBF driver). NC (basic) dataset.

db.describe census

doesn't report any error, it just hangs.

I have tried with r56943 on Windows8, still failing:

GRASS 7.0.svn> db.describe overpasses



FEHLER: Kann die Datenbank
        <$GISDBASE/$LOCATION_NAME/$MAPSET/sqlite/sqlite.db> nicht öffnen.

GRASS 7.0.svn> v.info -c overpasses
Displaying column types/names for database connection of layer <1>:
INTEGER|cat
CHARACTER|BRIDGE_NUM
CHARACTER|BSIP_BNUM
CHARACTER|STRUCTYPE
CHARACTER|FTR_INTRSC
CHARACTER|F_CARRIED
CHARACTER|COUNTY
GRASS 7.0.svn>

comment:34 in reply to:  33 ; Changed 7 years ago by hamish

Replying to neteler:

I have tried with r56943 on Windows8, still failing:

> GRASS 7.0.svn> db.describe overpasses
> 
> FEHLER: Kann die Datenbank
>         <$GISDBASE/$LOCATION_NAME/$MAPSET/sqlite/sqlite.db> nicht öffnen.
> 
> GRASS 7.0.svn> v.info -c overpasses
> Displaying column types/names for database connection of layer <1>:
> INTEGER|cat
> CHARACTER|BRIDGE_NUM
> CHARACTER|BSIP_BNUM
> CHARACTER|STRUCTYPE
> CHARACTER|FTR_INTRSC
> CHARACTER|F_CARRIED
> CHARACTER|COUNTY
> GRASS 7.0.svn>

is the overpass vector map in the current mapset? You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset, unless you explicitly set database= to the real path, not the default which may expand $MAPSET to the current mapset, not the other mapset's dir name.

Hamish

comment:35 in reply to:  34 Changed 7 years ago by hellik

Replying to hamish:

is the overpass vector map in the current mapset? You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset, unless you explicitly set database= to the real path, not the default which may expand $MAPSET to the current mapset, not the other mapset's dir name.

it seems the case here

v.info map=overpasses@PERMANENT                                                 
 +----------------------------------------------------------------------------+
 | Name:            overpasses                                                |
 | Mapset:          PERMANENT                                                 |
 | Location:        nc_spm_08_grass7                                          |
 | Database:        C:\grassdata                                              |
 | Title:           North Carolina NCDOT overpasses                           |
 | Map scale:       1:1                                                       |
 | Name of creator: NCDOT GIS, Raleigh, North Carolina                        |
 | Organization:    NC OneMap                                                 |
 | Source date:     January 2007                                              |
 | Timestamp (first layer): none                                              |
 |----------------------------------------------------------------------------|
 | Map format:      native                                                    |
 |----------------------------------------------------------------------------|
 |   Type of map: Vektor (level: 2)                                           |
 |                                                                            |
 |   Number of points:       17754           Number of centroids:  0          |
 |   Number of lines:        0               Number of boundaries: 0          |
 |   Number of areas:        0               Number of islands:    0          |
 |                                                                            |
 |   Map is 3D:              No                                               |
 |   Number of dblinks:      1                                                |
 |                                                                            |
 |   Projection: Lambert Conformal Conic                                      |
 |                                                                            |
 |               N:   317794.03225579    S:    14710.42725429                 |
 |               E:   929375.80949981    W:   126286.43250027                 |
 |                                                                            |
 |   Digitization threshold: 0                                                |
 |   Comment:                                                                 |
 |     Data from http://www.ncdot.org/it/gis/DataDistribution/ (Bridge Maps)  |
 +----------------------------------------------------------------------------+
g.mapset -p                                                                     
PERMANENT
db.describe overpasses                                                          
table:overpasses
description:
insert:?
delete:?
ncols:7
nrows:17754
column:cat
description:
type:INTEGER
len:20
scale:0
precision:0
default:
nullok:yes
select:?
update:?
[...]
g.mapset -p                                                                     
user1
db.describe overpasses                                                          
DBMI-SQLite driver error:
Error in sqlite3_prepare(): select * from overpasses where
oid < 0
no such table: overpasses
ERROR: Kann Tabelle <overpasses> nicht beschreiben.

comment:36 Changed 7 years ago by hamish

(which is the expected behaviour, since a table by that name doesn't exist in the current database, and db.connect is working on a raw table name, not the vector map's name. use v.db.connect to go through the vector map, which then via dbln points it to the linked database)

comment:37 Changed 7 years ago by mlennert

Is the original bug really still open ?

I would propose closing it and open new tickets for other issues.

Moritz

comment:38 in reply to:  34 ; Changed 7 years ago by neteler

Replying to hamish:

Replying to neteler:

I have tried with r56943 on Windows8, still failing:

> GRASS 7.0.svn> db.describe overpasses

...

is the overpass vector map in the current mapset? You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset, unless you explicitly set database= to the real path, not the default which may expand $MAPSET to the current mapset, not the other mapset's dir name.

In fact it was in a different mapset (PERMANENT). But it should not hang (instead it should report that the table was not found in the current mapset).

comment:39 in reply to:  38 ; Changed 7 years ago by mlennert

Replying to neteler:

Replying to hamish:

Replying to neteler:

I have tried with r56943 on Windows8, still failing:

> GRASS 7.0.svn> db.describe overpasses

...

is the overpass vector map in the current mapset? You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset, unless you explicitly set database= to the real path, not the default which may expand $MAPSET to the current mapset, not the other mapset's dir name.

In fact it was in a different mapset (PERMANENT). But it should not hang (instead it should report that the table was not found in the current mapset).

Does it really hand, or does it fail and give back the prompt after the error message ?

comment:40 in reply to:  39 Changed 7 years ago by mlennert

Replying to mlennert:

Replying to neteler:

Replying to hamish:

Replying to neteler:

I have tried with r56943 on Windows8, still failing:

> GRASS 7.0.svn> db.describe overpasses

...

is the overpass vector map in the current mapset? You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset, unless you explicitly set database= to the real path, not the default which may expand $MAPSET to the current mapset, not the other mapset's dir name.

In fact it was in a different mapset (PERMANENT). But it should not hang (instead it should report that the table was not found in the current mapset).

Does it really hand, or does it fail and give back the prompt after the error message ?

s/hand/hang

Moritz

comment:41 in reply to:  39 ; Changed 7 years ago by mlennert

Replying to mlennert:

Replying to neteler:

Replying to hamish:

Replying to neteler:

I have tried with r56943 on Windows8, still failing:

> GRASS 7.0.svn> db.describe overpasses

...

is the overpass vector map in the current mapset? You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset, unless you explicitly set database= to the real path, not the default which may expand $MAPSET to the current mapset, not the other mapset's dir name.

In fact it was in a different mapset (PERMANENT). But it should not hang (instead it should report that the table was not found in the current mapset).

Does it really hang, or does it fail and give back the prompt after the error message ?

ping

comment:42 in reply to:  41 ; Changed 7 years ago by neteler

Replying to mlennert:

Replying to mlennert:

Replying to neteler:

Replying to hamish:

Replying to neteler:

I have tried with r56943 on Windows8, still failing:

> GRASS 7.0.svn> db.describe overpasses

...

is the overpass vector map in the current mapset?

No. I tried from the user1 mapset.

You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset

Right. So we need a test for that rather than failing with a slightly obscured error message. Or expand it to say:

"Table <%s> not found in current mapset"

...

Does it really hang, or does it fail and give back the prompt after the error message ?

No, it comes back to the command line just checked again with a current OSGeo4W-GRASS 7.

comment:43 in reply to:  42 Changed 7 years ago by mlennert

Priority: blockernormal

Replying to neteler::

Replying to hamish:

You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset

Right. So we need a test for that rather than failing with a slightly obscured error message. Or expand it to say:

"Table <%s> not found in current mapset"

The current test is this:

    if (db_describe_table(driver, &table_name, &table) != DB_OK)
        G_fatal_error(_("Unable to describe table <%s>"), db_get_string(&table_name));

i.e. it justs checks whether it can describe the table and if not it fails. No attempt to no why it cannot describe the table (table does not exist in this mapset, bad spelling, etc). As the module does allow you to define the driver and database (meaning that you can actually describe a table in any database, not only tables in a database linked to a GRASS mapset), the issue is larger than just mapsets. I would suggest something like:

ERREUR :Unable to describe table <overpasses>. Check database connection
        information.

and adding a few lines to the manual.

I'm attaching a relevant patch proposal.

In any case, I'm downgrading this bug's priority.

Changed 7 years ago by mlennert

Attachment: db_describe.diff added

patch for better error message and more explanation in man page

comment:44 in reply to:  42 ; Changed 7 years ago by mmetz

Replying to neteler:

Replying to mlennert:

Replying to mlennert:

Replying to neteler:

Replying to hamish:

Replying to neteler:

I have tried with r56943 on Windows8, still failing:

> GRASS 7.0.svn> db.describe overpasses

...

is the overpass vector map in the current mapset?

No. I tried from the user1 mapset.

You may be seeing the effect of the db.* modules only working for databases linked from maps in the current mapset

This is not true. All db.* modules (should) have an option to define the database and driver, as for example db.describe, which can describe any table in any database, as long as the driver is supported.

Right. So we need a test for that rather than failing with a slightly obscured error message. Or expand it to say:

"Table <%s> not found in current mapset"

Better: "Table <%s> not found in database <%s> using driver <%s>"

Done in r57260.

comment:45 in reply to:  44 ; Changed 7 years ago by neteler

Replying to mmetz:

Replying to neteler:

Right. So we need a test for that rather than failing with a slightly obscured error message. Or expand it to say:

"Table <%s> not found in current mapset"

Better: "Table <%s> not found in database <%s> using driver <%s>"

Done in r57260.

Backported to 6.4 and 6.5 in r57264 and r57265.

Tested with DBF and SQLite backends:

# DBF driver, user1 mapset
GRASS 6.4.3svn (nc_spm_08):~/grass64 > db.describe bla
Table <bla> not found in database <$GISDBASE/$LOCATION_NAME/$MAPSET/dbf/>
using driver <dbf>

GRASS 6.4.3svn (nc_spm_08):~/grass64 > db.describe myarea
table:myarea
description:
insert:yes
delete:yes
ncols:5
nrows:1

----------
# SQLite driver, sqlite mapset
GRASS 6.4.3svn (nc_spm_08):~/grass64 > db.tables -p
testpoint

GRASS 6.4.3svn (nc_spm_08):~/grass64 > db.describe bla
Table <bla> not found in database

<$GISDBASE/$LOCATION_NAME/$MAPSET/sqlite.db> using driver <sqlite>
GRASS 6.4.3svn (nc_spm_08):~/grass64 > db.describe testpoint
table:testpoint
description:
insert:?
delete:?
ncols:4
nrows:1

column:cat
description:
type:INTEGER
...

comment:46 in reply to:  45 ; Changed 7 years ago by mlennert

Replying to neteler:

Replying to mmetz:

Replying to neteler:

Right. So we need a test for that rather than failing with a slightly obscured error message. Or expand it to say:

"Table <%s> not found in current mapset"

Better: "Table <%s> not found in database <%s> using driver <%s>"

Done in r57260.

Backported to 6.4 and 6.5 in r57264 and r57265.

Tested with DBF and SQLite backends:

Can we close this ticket, then ?

comment:47 in reply to:  46 ; Changed 7 years ago by neteler

Replying to mlennert:

Can we close this ticket, then ?

Shouldn't all relevant db.* be updated accordingly? It is yet only added to db.describe:

grep db_table_exists db/db.*/*
db/db.describe/main.c:    if (!db_table_exists(parms.driver, parms.database, parms.table)) {

comment:48 in reply to:  47 ; Changed 7 years ago by neteler

Replying to neteler:

Replying to mlennert:

Can we close this ticket, then ?

Shouldn't all relevant db.* be updated accordingly?

MM enhanced db.columns db.copy db.select in r57274.

Now backported to devel_branch6 in r57274. Relbranch TODO

comment:49 in reply to:  48 ; Changed 7 years ago by neteler

Replying to neteler:

Now backported to devel_branch6 in r57274. Relbranch TODO

Done for 6.4.4 in r57305.

comment:50 in reply to:  49 ; Changed 7 years ago by hamish

Replying to neteler:

Replying to neteler:

Now backported to devel_branch6 in r57274. Relbranch TODO

Done for 6.4.4 in r57305.

Hi,

fwiw & speaking generally, I would suggest to hold back on non-essential backporting to relbr64 for a couple of weeks in case an emergency 6.4.3.1 needs to be created from the release branch.

thanks, Hamish

comment:51 in reply to:  50 ; Changed 7 years ago by mlennert

Replying to hamish:

Replying to neteler:

Replying to neteler:

Now backported to devel_branch6 in r57274. Relbranch TODO

Done for 6.4.4 in r57305.

Hi,

fwiw & speaking generally, I would suggest to hold back on non-essential backporting to relbr64 for a couple of weeks in case an emergency 6.4.3.1 needs to be created from the release branch.

We're talking about changes to error messages to make them more useful for users. I don't think this kind of backport has to be held back...

Let's have these kind of corrections in 6.4.3 instead of having to wait for a hypothetical 6.4.3.1.

I think that this bug can now be closed, or ?

Moritz

comment:52 in reply to:  51 Changed 7 years ago by hamish

Resolution: fixed
Status: newclosed

Replying to mlennert:

We're talking about changes to error messages to make them more useful for users. I don't think this kind of backport has to be held back...

it's a moot point now, but fwiw *any* change in the last hours before a release is highly dangerous, no matter how safe it appears.

best, H

Note: See TracTickets for help on using tickets.