[GRASS5] Re: [bug #2767] (grass) r.stats bug (due to recent

On Wed, Dec 08, 2004 at 09:51:08PM +0100, Paul Kelly via RT wrote:

Hello Thierry

On Tue, 7 Dec 2004 tlaronde@polynum.com wrote:

> Hello,
>
> I haven't look at the code you are trying to fix (I have some other
> stuff ;-), but here are some tips :
>
[...]
Snip explanation of why my suggested fix was completely wrong
[...]
>
> So a correct sequence is :
>
> title = (char *) malloc(expected_size_of_string_plus_trailing_null);
> *title = '\0'; /* default to empty if intitle not set */
>

The first line here is more or less what G_store() does. But the recent
change added
if ( s == NULL ) return NULL ;
to G_store.

This change was needed to make db.login happy
(locally discussed with Radim).

Perhaps (going by what you suggest above) it should look more like:
char *G_store (char *s)
{
   char *buf;

   if ( s == NULL )
     *buf = '\0';
   else
   {
     buf = G_malloc (strlen(s) + 1);
     strcpy (buf, s);
   }
   return buf;
}

... also crashing:

(gdb) r -anC fields,elevation.10m
Starting program: /hardmnt/thuille0/ssi/software/cvsgrass57/dist.i686-pc-linux-gnu/bin/r.stats -anC fields,elevation.10m

Program received signal SIGSEGV, Segmentation fault.
0x00306657 in G_set_raster_cats_title (title=0xbfffb3c0 "", pcats=0x7a2d4fd0) at cats.c:1536
1536 pcats->title = G_store (title);
(gdb) bt
#0 0x00306657 in G_set_raster_cats_title (title=0xbfffb3c0 "", pcats=0x7a2d4fd0) at cats.c:1536
#1 0x0030657c in G_init_raster_cats (title=0xbfffb3c0 "", pcats=0x7a2d4fd0) at cats.c:1488
#2 0x00304fcb in G__read_cats (element=0x340840 "cats", name=0x94a0520 "elevation.10m",
    mapset=0x94a0218 "PERMANENT", pcats=0x7a2d4fd0, full=1) at cats.c:488
#3 0x00304cb5 in G_read_raster_cats (name=0x94a0520 "elevation.10m",
    mapset=0x94a0218 "PERMANENT", pcats=0x7a2d4fd0) at cats.c:372
#4 0x00304c80 in G_read_cats (name=0x94a0520 "elevation.10m", mapset=0x94a0218 "PERMANENT",
    pcats=0x7a2d4fd0) at cats.c:350
#5 0x0804a402 in main (argc=3, argv=0xbfffbb24) at main.c:269

Is it possible that 'pcats' isn't initialized properly?

If I run it in 'ddd' and add a break point to the first line of G_init_raster_cats()
in lib/gis/cats.c:
G_init_raster_cats (char *title, struct Categories *pcats)
{
    G_set_raster_cats_title (title, pcats);

just before segfaulting, pcats isn't initialized.

So maybe G__read_cats() in G_read_raster_cats() called by G_read_cats()
already fails? In G__read_cats() the function G_init_raster_cats() is executed,
but pcats is reported as 'disabled' in the ddd debugger at that state.

so you are returning a pointer to an empty string rather than returning a
NULL pointer. But I am still not very hopeful.

The funny thing is I am unable to reproduce this bug so cannot test.
Running 'r.stats -anC fields' in Spearfish with yesterday's 5.7 CVS on IRIX
produces normal-looking output.

Also here.
The problem appears with

r.stats -anC fields,elevation.10m
Segmentation fault

Markus

On Thu, Dec 09, 2004 at 10:35:16AM +0100, Markus Neteler wrote:

... also crashing:

(gdb) r -anC fields,elevation.10m
Starting program: /hardmnt/thuille0/ssi/software/cvsgrass57/dist.i686-pc-linux-gnu/bin/r.stats -anC fields,elevation.10m

Program received signal SIGSEGV, Segmentation fault.
0x00306657 in G_set_raster_cats_title (title=0xbfffb3c0 "", pcats=0x7a2d4fd0) at cats.c:1536
1536 pcats->title = G_store (title);

This is the call to G_store (see the previous answer to Paul) since with
the code:

char *buf;

if (s == NULL)
  *buf = '\0';

one is still writing the char '\0' to an unknown address since buf has
a random value (probably 0 in this case, leading to the SEGFAULT; and
no SEGFAULT on a system where buf, not initialized, happens to point
to some valid address---thus trashing some place it doesn't own).

So correct sequence here too: initialize (reserve address) before
writing to it:

if (s == NULL)
  buf = (char *) calloc(1,1); /* 1 byte allocated and destination zeroed */

( same as doing:
buf = (char *) malloc(1);
*buf = '\0';
)

But I still have the intuition that you should test in r.stats, and not
change G_store().

If the program is still segfaulting with such a change, there is another
bug somewhere else...

Cheers,
--
Thierry Laronde (Alceste) <tlaronde +AT+ polynum +dot+ com>
http://www.kergis.org/ | http://www.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89 250D 52B1 AE95 6006 F40C