Hi,
to improve d.area -f not to use the monitor background color
for polygon filling (then these polygons appear to be non-existent)
I try to catch the current monitor background color.
However, it doesn't work for me (I may be lost again with strings
and pointers):
char * colorname;
int bgcolor;
bgcolor=D_translate_color(D_get_erase_color(colorname));
seg fault... Sorry for this dump question.
Markus
PS: the display functions seem to be spreaded along libes/D/ and
libes/display/
Markus Neteler writes:
> However, it doesn't work for me (I may be lost again with strings
> and pointers):
>
> char * colorname;
> int bgcolor;
>
> bgcolor=D_translate_color(D_get_erase_color(colorname));
Markus, the code above is broken. You're passing an uninitialized
pointer (colorname), to D_get_erase_Color. Inside, that function does:
strcpy(colorname, list[0]);
which is sure to segfault or worse if colorname is not pointing to
allocated memory.
Instead, you should first ensure that colorname is pointing to some
allocated memory large enough to hold the resulting color. Something
like:
char colorname[MAX_COLOR_LEN];
I have no idea what value might be appropriate for MAX_COLOR_LEN.
(And if there isn't a well-defined value for this somewhere, then
D_get_erase_color should be updated to accept a maximum length as an
input parameter or to allocate memory for the result itself).
-Carl
On Fri, Oct 26, 2001 at 09:48:15AM -0400, Carl Worth wrote:
Markus Neteler writes:
> However, it doesn't work for me (I may be lost again with strings
> and pointers):
>
> char * colorname;
> int bgcolor;
>
> bgcolor=D_translate_color(D_get_erase_color(colorname));Markus, the code above is broken. You're passing an uninitialized
pointer (colorname), to D_get_erase_Color. Inside, that function does:strcpy(colorname, list[0]);
which is sure to segfault or worse if colorname is not pointing to
allocated memory.Instead, you should first ensure that colorname is pointing to some
allocated memory large enough to hold the resulting color. Something
like:char colorname[MAX_COLOR_LEN];
I have no idea what value might be appropriate for MAX_COLOR_LEN.
(And if there isn't a well-defined value for this somewhere, then
D_get_erase_color should be updated to accept a maximum length as an
input parameter or to allocate memory for the result itself).
Thanks for the hints, Carl,
however, I am still a bit stuck here. Perhaps our display gurus
have a recommendation?
Thanks,
Markus
Markus Neteler wrote:
> Instead, you should first ensure that colorname is pointing to some
> allocated memory large enough to hold the resulting color. Something
> like:
>
> char colorname[MAX_COLOR_LEN];
>
> I have no idea what value might be appropriate for MAX_COLOR_LEN.
>
> (And if there isn't a well-defined value for this somewhere, then
> D_get_erase_color should be updated to accept a maximum length as an
> input parameter or to allocate memory for the result itself).
>Thanks for the hints, Carl,
however, I am still a bit stuck here. Perhaps our display gurus
have a recommendation?
There isn't a maximum length; D_get_erase_color() returns whatever was
passed to D_get_erase_color().
Certainly, Carl is right that functions which accept pointers to
"output" arrays (ones into which results are written) should also
accept a size argument. But then how to deal with overflows?
Truncating the result to fit prevents crashes, but may cause more
subtle errors. G_fatal_error() isn't particularly friendly. Having the
caller enlarge the buffer then try again is the right approach, but
probably won't happen. Better to use G_store() and return a pointer;
that way, the worst case is a memory leak.
Also, I suspect that this is far from the only place in GRASS where
this occurs. GRASS is littered with fixed-size buffers; in most cases,
it's just assumed that the buffer will be large enough.
--
Glynn Clements <glynn.clements@virgin.net>
Glynn Clements writes:
> Certainly, Carl is right that functions which accept pointers to
> "output" arrays (ones into which results are written) should also
> accept a size argument. But then how to deal with overflows?
[...]
> Better to use G_store() and return a pointer; that way, the worst
> case is a memory leak.
I agree this is probably the right approach to take.
> Also, I suspect that this is far from the only place in GRASS where
> this occurs. GRASS is littered with fixed-size buffers; in most cases,
> it's just assumed that the buffer will be large enough.
Yes, this does appear common. And fixing it means changing the API and
changing *lots* of code, but it could prevent some errors that would
be very difficult to track down. Another nice benefit would be that it
could eliminate some arbitrary length limits that currently exist in
GRASS. Limitations like this can be particularly annoying, (eg. I
really wish that I could use much longer names for my locations).
As long as we're talking about incompatible changes to the code
base... What other big changes are planned for the GRASS code? Here
are some I would like to see, (I know that I have seen a few of these
mentioned on the list before).
Elimination of arbitrary size limits
Revamped build system
Elimination of "single-session" nature of GRASS
Better support for temporal data
Better GUI, (particularly for a handheld with a small display,
a 1-button pointer and no keyboard)
Others?
Are any of these changes in progress or at least planned out? Are
there web pages outlining things like this?
I'm currently "under-the-gun" to get a working system up using GRASS
for a big demo at the end of November. After that is over, I should be
in a position to devote a lot of time to GRASS development. I have a
particular interest in the temporal support and the improved UI for
handhelds, but I can imagine myself responding to an itch strong
enough to start tackling the others -- unless someone beats me to it
of course :-).
-Carl
--
Carl Worth
USC Information Sciences Institute cworth@east.isi.edu
3811 N. Fairfax Dr. #200, Arlington VA 22203 703-812-3725
On Mon, Oct 29, 2001 at 10:46:08AM -0500, Carl Worth wrote:
Glynn Clements writes:
> Certainly, Carl is right that functions which accept pointers to
> "output" arrays (ones into which results are written) should also
> accept a size argument. But then how to deal with overflows?[...]
> Better to use G_store() and return a pointer; that way, the worst
> case is a memory leak.I agree this is probably the right approach to take.
> Also, I suspect that this is far from the only place in GRASS where
> this occurs. GRASS is littered with fixed-size buffers; in most cases,
> it's just assumed that the buffer will be large enough.Yes, this does appear common. And fixing it means changing the API and
changing *lots* of code, but it could prevent some errors that would
be very difficult to track down. Another nice benefit would be that it
could eliminate some arbitrary length limits that currently exist in
GRASS. Limitations like this can be particularly annoying, (eg. I
really wish that I could use much longer names for my locations).
in general I agree. The problem is that we don't want to break
5.0 again 
Why not starting 5.1 now?
As long as we're talking about incompatible changes to the code
base... What other big changes are planned for the GRASS code? Here
are some I would like to see, (I know that I have seen a few of these
mentioned on the list before).Elimination of arbitrary size limits
Revamped build system
Elimination of "single-session" nature of GRASS
Better support for temporal data
Better GUI, (particularly for a handheld with a small display,
a 1-button pointer and no keyboard)
Others?Are any of these changes in progress or at least planned out? Are
there web pages outlining things like this?
Yes, a draft is here:
http://grass.itc.it/grass51/index.html
-> Milestones.
In CVS:
documents/grass51milestones.html
Feel free to update that page, add more ideas etc.
I'm currently "under-the-gun" to get a working system up using GRASS
for a big demo at the end of November. After that is over, I should be
in a position to devote a lot of time to GRASS development. I have a
particular interest in the temporal support and the improved UI for
handhelds, but I can imagine myself responding to an itch strong
enough to start tackling the others -- unless someone beats me to it
of course :-).-Carl
Great!
Best regards
Markus
Carl Worth wrote:
> Also, I suspect that this is far from the only place in GRASS where
> this occurs. GRASS is littered with fixed-size buffers; in most cases,
> it's just assumed that the buffer will be large enough.Yes, this does appear common. And fixing it means changing the API and
changing *lots* of code, but it could prevent some errors that would
be very difficult to track down. Another nice benefit would be that it
could eliminate some arbitrary length limits that currently exist in
GRASS. Limitations like this can be particularly annoying, (eg. I
really wish that I could use much longer names for my locations).
Security is another reason for fixing it. Several people have, at some
point, talked about using GRASS as the back-end of a web-based system.
As things stand, any web interface had better check its input very
carefully, otherwise it will create a gaping security hole.
--
Glynn Clements <glynn.clements@virgin.net>
On Wed, Oct 31, 2001 at 11:23:56PM +0000, Glynn Clements wrote:
Basically, choose a value for the longest colour name which is likely
to be used as the erase colour. Currently, this is 8 bytes ("magenta"
is 7, plus the terminating NUL). Even allowing for future expansion, a
32-byte buffer should be more than sufficient (the longest colour name
in rgb.txt is LightGoldenrodYellow, which is 20 characters).
So, back on d.area:
I have added in src/includes/colors.h
#define MAX_COLOR_LEN 32 /* max length of color string */
#define MAX_COLOR_NUM 13 /* number of coded colors */
and implemented the background color filter for "d.area -f" mode.
Now all areas are filled when using -f, but the monitor background
color is omitted. The prevents confusion as otherwise areas filled
with monitor background color seemed to be non-existing.
Let me know if problems occur,
cheers
Markus