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

[Ivan Shmakov]

>>>>> Glynn Clements <glynn at gclements.plus.com> writes:

 >>>> 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.

	There're a plenty of cases where such checks are necessary
	regardless of the content length check being implemented.

	I'd consider it an error if, say, the value of D_get_cell_name
	() not being checked for being non-negative.

 >> 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).

	I'd have that check anyway.  However, yes, it's certainly of low
	priority, and I won't insist on the change above being applied;
	I'd rather keep this patch around in case anyone will ever need
	it.

[...]