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

[tlaronde at polynum.com]

Hello,

I haven't look at the code you are trying to fix (I have some other
stuff ;-), but here are some tips :

On Tue, Dec 07, 2004 at 10:27:14PM +0100, Markus Neteler wrote:
>  
> > If it doesn't something like this might:
> > int 
> > G_set_raster_cats_title (char *intitle, struct Categories *pcats)
> > {
> >     char *title;
> >     if (intitle == NULL)
> >       *title="";
> >     else
> >       title=intitle;
> >     pcats->title = G_store (title);
> >     G_newlines_to_spaces (pcats->title);
> >     G_strip (pcats->title);
> >       return 0;
> > }

title is declared as a char*
title is supposed to hold a value char*, but is current value after the
declaration in the local scope of the function is random (writing to
this destination will cause a mess).

The assignement :

title = "";

is syntactically correct since one affects to a char * a char * ("" is
indeed a pointer to a string reduced to one char '\0'). 
But here title is initialized
with the memory address of the "" string, which is whether put by the
compiler
in read-only memory (since "" is not a variable and doesn't change) or
may be allocated on the stack by some instructions generated by the
compiler (depending on optimization), thus making title points to a
read-only address or an evanescent address (on the stack).
Trying to change the value of the char pointed to by the address of
"", hence the address of title (the char '\0') afterwards is likely to
cause problem. 

The assignement :
*title = "";

is syntactically and logically wrong : one is assigning a pointer ("" is
a pointer to the empty string) to an integer value (a char, since *title
is a char because title is a pointer to char). Furthermore, since title
is not initialized, one is trying to write something in a random address
(that may be initialized to NULL, but this is not guaranteed for
automatic variables at all).

So a correct sequence is :

title = (char *) malloc(expected_size_of_string_plus_trailing_null);
*title = '\0'; /* default to empty if intitle not set */

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