>>>>> Glynn Clements <[EMAIL PROTECTED]> writes:
>> As of a recent SVN HEAD, `d.legend' uses a static buffer to store
>> the value of the `map' option. It's therefore impossible to pass
>> raster names more than 63 bytes long to `d.legend'. Since I don't
>> see why a static buffer may be necessary here, I suggest the
>> following (yet untested) patch.
[...]
>> PS. I've seen a number of other uses of static buffers scattered
>> across the code, leading to both potential limits and crashes, which
>> I'm on the way to investigate.
> There isn't necessarily a problem with using fixed-size buffers for
> map names, but the size should be GNAME_MAX.
Well, yes. It's actually strcpy () use which bothers me the
most, and not the static buffer per se. Obviously, `d.legend'
may accidentally get called with a `map' value larger than any
predefined static buffer size, leading to hardly predicable
behaviour.
In my opinion, strncpy () should generally be preferred over
strcpy ().
> Similar constants exist for mapset names and pathnames:
> /* File/directory name lengths */
> #define GNAME_MAX 256
> #define GMAPSET_MAX 256
ACK, thanks for the info.
What I'm talking about actually is the use of strcpy () in
lib/display/list.c. Apparently, the static buffers passed to,
e. g., D_get_cell_name () have the sizes that don't match any of
the constants above. Consider, e. g.:
$ nl -ba display/d.colors/main.c
...
25 int
26 main (int argc, char **argv)
27 {
28 char name[128] = "";
...
43 if (R_open_driver() == 0)
44 {
45 if(D_get_cell_name (name) < 0)
46 *name = 0;
47 R_close_driver();
48 }
...
$
Doesn't the above mean that `d.colors' cannot handle rasters
with names more than 127 bytes long?
> #define GPATH_MAX 4096
... Though any such limits give me a hard feeling in general.
Consider, e. g., GNU/Hurd system, which has no limits on the
path length. Wouldn't the above involve extra effort to the
porters?
> In the specific case of d.legend, your fix is the correct one,
> except that map_name needs to be "char *" not "const char *", as
> G_find_cell() may modify the map name (it removes any @mapset
> suffix).
ACK. I've suspected the issues like this one. I guess, would I
make a test build of GRASS, the compiler would issue a warning.
> As this never enlarges the string, it's okay to re-use the original
> buffer.
I see, Martin has turned G_find_cell () into G_find_cell2 ()
instead.
_______________________________________________
grass-dev mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/grass-dev
