[GRASS-dev] d.legend uses a static buffer for `map='

[Glynn Clements]

Ivan Shmakov wrote:

>  >> In my opinion, strncpy () should generally be preferred over strcpy
>  >> ().
> 
>  > Using strncpy() isn't much of an improvement, as the copy won't be
>  > NUL-terminated if it's truncated. And simplyy NUL-terminating the
>  > string risks reading/writing the wrong file. The only safe solutions
>  > are to either dynamically allocate all buffers, or signal a fatal
>  > error if the source is too long.
> 
> 	Or, if a library function is considered, it may return a kind of
> 	error to the caller.

That requires each caller to check for errors.

> 	For example, I'd suggest changing D_get_cell_name () as follows:
> 
> diff --git a/lib/display/list.c b/lib/display/list.c
> index 48180cf..2353092 100644
> --- a/lib/display/list.c
> +++ b/lib/display/list.c
> @@ -99,6 +99,9 @@ int D_get_cell_name(char *name )
>  	if ((stat = R_pad_get_item ("cell", &list, &count)))
>  		return(-1) ;
>  
> +	if (strlen (list[0]) >= GNAME_MAX)
> +		return -1;
> +

That isn't a problem; it's reasonably safe to assume that the names
which appear in the list don't exceed that value (e.g. most
filesystems won't let you have a name longer than that).

>  >> $ nl -ba display/d.colors/main.c 
>  >> ...
>  >>     25	int 
>  >>     26	main (int argc, char **argv)
>  >>     27	{
>  >>     28	    char name[128] = "";
> 
>  > Right. We encourage developers to replace these with the defined
>  > constants as they encounter them.
> 
> 	ACK.

Well, it's not as if anyone is going to sytematically examine the
entire GRASS source code to find such cases.

> PS.  Shouldn't G__open () use GPATH_MAX as well?

Yes. And GNAME_MAX and GMAPSET_MAX.

And it shouldn't be freeing the mapset either.

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