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

[Glynn Clements]

Ivan Shmakov wrote:

> 	Could you review the attached patch?

Seems fine.

> 	Things I didn't change with this patch:
> 
> 	* buffers related to SQL commands, because I know no appropriate
> 	  Cpp macro for these;

I would suggest just adding e.g.:

	#define SQL_COMMAND_MAX 4000

to the top of the file, then using that. Although it doesn't provide
consistency between modules, it makes it easier to find such constants
if they're at the top of the file.

> 	* buffers related to command lines to be passed to system (),
> 	  since I believe that using a specific library to construct the
> 	  arguments lists to be passed to an `exec' family function
> 	  directly would be a better solution; (check the `man-db'
> 	  source [1] for a suitable facility);

I've already done some work on eliminating the use of system(); see
lib/gis/spawn.c. I'd rather see more code modified to use those
functions than just tinkering with the buffer size.

> 	* misused G_warning (), etc.; like:
> 
>    char BUF[...];
>    sprintf (BUF, FORMAT, ...);
>    G_warning (BUF);
> 
> 	  instead of:
> 
>    G_warning (FORMAT, ...);
> 
> 	  (the documentation should emphasize the first argument being
> 	  the printf ()-style format string, and /not/ the message);
> 	  there're just too many of these and I hope to handle them with
> 	  a script;

Note that those functions are tagged with:

	__attribute__((format(printf,1,2)));

With gcc, you can use the option:

`-Wformat-nonliteral'
     If `-Wformat' is specified, also warn if the format string is not a
     string literal and so cannot be checked, unless the format function
     takes its format arguments as a `va_list'.

to identify problematic cases (although --with-nls will interfere with
this).

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