Opened 16 years ago
Closed 6 years ago
#461 closed defect (fixed)
v.to.db crashes on a shapefile connected with v.external
Reported by: | msieczka | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 7.0.7 |
Component: | Vector | Version: | svn-trunk |
Keywords: | v.external, v.to.db | Cc: | martinl |
CPU: | x86-64 | Platform: | Linux |
Description (last modified by )
Backtrace:
GRASS 6.5.svn (javier):~ > gdb v.to.db GNU gdb 6.8-debian Copyright (C) 2008 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as https://trac.osgeo.org/grass/ticket/461#no3"x86_64-linux-gnu"... (gdb) run map=gshhs_shp opt=area units=k col=AREA Starting program: /usr/local/grass-6.5.svn/bin/v.to.db map=gshhs_shp opt=area units=k col=AREA [Thread debugging using libthread_db enabled] warning: Lowest section in /usr/lib/libicudata.so.38 is .hash at 0000000000000120 [New Thread 0x7fe8946c9710 (LWP 9814)] Reading areas... 100% Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fe8946c9710 (LWP 9814)] 0x00007fe89418f539 in db_get_column_sqltype (column=0x0) at column.c:107 107 return column->sqlDataType; (gdb) bt #0 0x00007fe89418f539 in db_get_column_sqltype (column=0x0) at column.c:107 #1 0x00007fe8938f480d in db_select_int (driver=0x9237ba0, tab=0x98bb7f0 "gshhs", col=0x98b8ba0 "", where=0x0, pval=0x7fff9c7ff858) at select.c:138 #2 0x00000000004069f2 in update (Map=0x7fff9c7ff8c0) at update.c:53 #3 0x00000000004043e4 in main (argc=5, argv=0x7fff9c7fffd8) at main.c:84
Change History (18)
follow-up: 2 comment:1 by , 15 years ago
comment:2 by , 15 years ago
Summary: | v.db.update crashes with updating shapefile connected with v.external → v.to.db crashes on a shapefile connected with v.external |
---|
Replying to mlennert:
The title of this bug concerns v.db.update, but the backtrace is on v.to.db.
My bad. Correcting the title.
Maciej, could you detail this a bit more ?
- run v.external on some shapefile
- try to populate the shapefile's datatable with v.to.db - crash
comment:3 by , 15 years ago
The manual http://grass.osgeo.org/grass64/manuals/html64_user/v.external.html says: Creates a new vector as a read-only link to OGR layer. Apparently some error trapping is missing which tells the user that v.external is a read-only link.
comment:4 by , 15 years ago
Do we have a function in order to test if an attribute table is open for writing?
follow-up: 6 comment:5 by , 15 years ago
[copied from the ML]
-> still valid?
Markus Metz wrote:
I think so because attribute queries are not possible on OGR vectors connected with v.external (most OGR vector layers don't have a grass-like key column). I think Martin L fixed that in grass7.
...
Attribute table operations (query, upload values) are still not possible in grass7 for OGR vector layers that don't have a key column, i.e. for most OGR-supported formats. Still valid for both v.external and direct OGR link.
We may not be able to add the requested functionality for the next release, but can we do anything about replacing the segfault with a G_fatal_error()?
Hamish
follow-up: 8 comment:6 by , 15 years ago
Replying to hamish:
We may not be able to add the requested functionality for the next release, but can we do anything about replacing the segfault with a G_fatal_error()?
The following is based upon the backtrace, and is completely untested:
--- lib/db/dbmi_client/select.c (revision 40258) +++ lib/db/dbmi_client/select.c (working copy) @@ -134,6 +134,8 @@ table = db_get_cursor_table(&cursor); column = db_get_table_column(table, 0); /* first column */ + if (!column) + G_fatal_error(_("Table has no columns")); value = db_get_column_value(column); type = db_get_column_sqltype(column); type = db_sqltype_to_Ctype(type);
comment:7 by , 15 years ago
The suggested patch looks good:
# NC dataset v.external medfacs.shp out=medfacs layer=medfacs # unpatched: GRASS 6.4.0svn (nc_spm_08):~/books/kluwerbook/data3rd/wake > v.to.db medfacs option=area columns=AREA Reading areas... Segmentation fault # patched: GRASS 6.4.0svn (nc_spm_08):~/books/kluwerbook/data3rd/wake > v.to.db medfacs option=area columns=AREA Reading areas... ERROR: Table has no columns
It may not be immediate to derive the problem in this particular case but a healthy test. Please submit and I'll backport.
follow-up: 9 comment:8 by , 15 years ago
Replying to glynn:
Replying to hamish:
We may not be able to add the requested functionality for the next release, but can we do anything about replacing the segfault with a G_fatal_error()?
The following is based upon the backtrace, and is completely untested:
--- lib/db/dbmi_client/select.c (revision 40258) +++ lib/db/dbmi_client/select.c (working copy) @@ -134,6 +134,8 @@ table = db_get_cursor_table(&cursor); column = db_get_table_column(table, 0); /* first column */ + if (!column) + G_fatal_error(_("Table has no columns")); value = db_get_column_value(column); type = db_get_column_sqltype(column); type = db_sqltype_to_Ctype(type);
Going a bit further to the root of the problem: in this case const char *col passed to db_select_int() in lib/db/dbmi_client/select.c is a zero length string, you could do for 'col' the same test that's done for 'where' and exit with an error.
Even further down, the question is, what to do if OGR_L_GetFIDColumn() doesn't return FID name, to cite a comment from Vect_read_dblinks().
Markus M
comment:9 by , 15 years ago
Replying to mmetz:
Going a bit further to the root of the problem: in this case const char *col passed to db_select_int() in lib/db/dbmi_client/select.c is a zero length string, you could do for 'col' the same test that's done for 'where' and exit with an error.
The following patch works for me
Index: select.c =================================================================== --- select.c (revision 40416) +++ select.c (working copy) @@ -115,6 +115,9 @@ G_debug(3, "db_select_int()"); + if (col == NULL || strlen(col) == 0) + G_fatal_error(_("Missing column name")); + /* allocate */ alloc = 1000; val = (int *)G_malloc(alloc * sizeof(int)); @@ -203,6 +206,12 @@ dbValue *value; dbTable *table; + if (key == NULL || strlen(key) == 0) + G_fatal_error(_("Missing key column name")); + + if (col == NULL || strlen(col) == 0) + G_fatal_error(_("Missing column name")); + G_zero(val, sizeof(dbValue)); sprintf(buf, "SELECT %s FROM %s WHERE %s = %d\n", col, tab, key, id); db_init_string(&stmt); @@ -259,6 +268,12 @@ G_debug(3, "db_select_db_select_CatValArray ()"); + if (key == NULL || strlen(key) == 0) + G_fatal_error(_("Missing key column name")); + + if (col == NULL || strlen(col) == 0) + G_fatal_error(_("Missing column name")); + db_init_string(&stmt); sprintf(buf, "SELECT %s, %s FROM %s", key, col, tab);
Problem is when a module gives a broken sql string to db_execute_immediate(), hopefully that aborts with an error and not a segfault.
Glynn's patch could be useful in other circumstances too.
Markus M
comment:10 by , 15 years ago
Regarding the recent discussion about GRASS libraries and G_fatal_error() as well as the description of these functions, maybe rather issue a warning instead of an error and return -1? That would work because (at least) v.to.db checks the return value of db_select_int() and exits with an error if db_select_int() returns -1.
Markus M
comment:11 by , 15 years ago
comment:12 by , 15 years ago
Keywords: | v.external v.to.db added |
---|---|
Milestone: | 6.4.0 → 7.0.0 |
Priority: | critical → major |
Version: | svn-develbranch6 → svn-trunk |
comment:13 by , 11 years ago
What is the status of this bug ? Any chance v.external will ever offer write access to attribute tables of OGR-linked vectors ? With a recent grass7 I still get the "Missing column name" message. I actually do not find that message very clear. My first reaction was to check whether I had forgotten to indicate the column I wanted to upload to. Maybe some hint about linked maps would be helpful.
comment:14 by , 11 years ago
Cc: | added |
---|
comment:15 by , 8 years ago
Milestone: | 7.0.0 → 7.0.5 |
---|
comment:16 by , 8 years ago
Milestone: | 7.0.5 → 7.0.6 |
---|
comment:17 by , 6 years ago
Milestone: | 7.0.6 → 7.0.7 |
---|
comment:18 by , 6 years ago
Description: | modified (diff) |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Originally reported bug seems to be fixed. Free free to reopen if needed.
The title of this bug concerns v.db.update, but the backtrace is on v.to.db.
Maciej, could you detail this a bit more ?
Moritz