Ivan Shmakov wrote:
> >> Is a new D_number_to_RGB(int color, unsigned char &r,&g,&b) fn
> >> needed in lib/display/tran_colr.c?
>
> > Yes. E.g.:
>
> > int D_color_number_to_RGB(int color, int &r, int &g, int &b)
> > {
>
> That looks a lot like C++. Surely it will work with C89?
Oops. I meant:
int D_color_number_to_RGB(int color, int *r, int *g, int *b)
> > const struct color_rgb *c;
>
> > if (color <= 0)
> > return 0;
>
> BTW, is it a GRASS convention to use 0 to indicate failure?
> Standard C library functions tend to use -1 for failure, while
> using non-negative values to indicate success.
The GRASS convention is to use a different convention for each
function ;)
More seriously: functions which only return a success/failure
indication typically (but not always) return 1 for success and 0 for
failure, so that conditionals read naturally, i.e. "if (foo(...))"
means "if it succeeds" and "if (!foo(...))" means "if it fails".
Functions which normally return a non-negative integer normally return
a negative integer to indicate failure.
Functions which return 0 for success should ideally be tested using
"if (foo(...) == 0)" rather than "if (!foo(...))", as the use of "!"
connotes a negative (i.e. failure).
> Also, it seems feasible to set `errno' in the library functions
> like this. Do I understand correctly that it isn't done (with
> any regularity) in GRASS?
AFAIK, GRASS itself never sets errno.
> > if (color >= ncolors)
> > return 0;
>
> > c = &colors[color];
> > r = c->r;
> > g = c->g;
> > b = c->b;
>
> > return 1;
> > }
>
> [...]
>
> I'd suggest the following, but I don't know (yet) the
> appropriate conventions for the D_* functions.
>
> int
> D_color_number_to_RGB (int color, int *r, int *g, int *b)
> {
> const struct color_rgb *c;
>
> if (color <= 0 || color >= ncolors) {
> errno = EINVAL;
> return -1;
> }
>
> c = &colors[color];
> if (r != 0) *r = c->r;
> if (g != 0) *g = c->g;
> if (b != 0) *b = c->b;
>
> return 0;
> }
>
> With the `if (X != 0)' guards, the function may be used to query
> whether the given color index exists, while not asking for its
> components to be returned, like:
>
> if (D_color_number_to_RGB (INDEX, 0, 0, 0) < 0) {
> /* color INDEX doesn't exist */
> }
That makes sense, although I would omit the "!= 0", and just use e.g.
"if (r) ...". Generally, I prefer "if (x)" and "if (!x)" if zero/NULL
is a false/unset/etc indicator.
> If `struct color_rgb' is a ``public'' data type, then it might
> be appropriate to code this function as:
The problem is that we have both color_rgb (colors.h) and RGBA_Color
(gis.h) structures, and we should deprecate one of them (probably
color_rgb).
--
Glynn Clements <[EMAIL PROTECTED]>
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev
