[GRASS-dev] lib fn to convert int color number to RGB values?

Ivan Shmakov ivan at theory.asu.ru
Thu Jan 31 08:08:27 EST 2008


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

[...]

 >>> If it's likely that the module is just going to call
 >>> G_fatal_error() anyhow, the library function may as well call it
 >>> itself, thereby providing a "infallible" interface (i.e. the
 >>> function never fails; it either succeeds, or the process is no
 >>> longer running so the failure is moot).

 >> [...]

 >>> The former is only an advantage if there's a situation where a
 >>> module doesn't just call G_fatal_error() when an invalid colour
 >>> occurs. Given that invalid user input is usually a fatal error, I
 >>> can't immediately think of a situation where you might get an
 >>> invalid colour string without it resulting in a fatal error.

 >> Did you consider the use of the function from within a GUI
 >> application?  Hardly calling exit () on incorrect input from the
 >> user is a friendly behaviour for a GUI application.

 > GRASS simply doesn't work in such a context. One more function
 > doesn't make any difference unless you're going to fix the thousands
 > that already behave this way.

	Well, I haven't looked at the code just yet, but it seems that
	the references to the functions in question aren't scattered too
	widely across the library:

$ find lib/ -name \*.o -print0 \
      | xargs -r0 -- nm -- \
      | grep -E 'U (_?exit|G_fatal_error)$' \
      | LC_ALL=C sort | uniq -c 
     97          U G_fatal_error
      4          U _exit
     39          U exit
$ find lib/ -name \*.o | wc -l 
811
$ 

	(Yes, there're probably a number of indirect callers as well.)

 > BTW, we seem to have had this discussion at least once a year for as
 > long as I have been subscribed to these lists (~7 years).

	I'm glad that I'm not the first one to discover that there's a
	plenty of room for future improvement.

[...]

 > Apart from calling exit() via G_fatal_error()[1], there are some
 > other obstacles to making the GRASS libraries behave like normal
 > libraries.  The most significant is the amount of static data which
 > cannot be multiply instantiated, most of which is initialised at
 > first use and often cannot be reset or otherwise modified thereafter.

 > The GRASS libraries are fundamentally designed to be used in
 > "one-shot" modules. Errors are handled by printing a message then
 > exiting. Variables are either initialised at startup from the .data
 > and .bss segments or are initialised upon first use. Memory doesn't
 > get explicitly de-allocated, as the operating system will do that
 > when the module terminates.

	Thanks for the clarification, but isn't the main question in
	whether there's any interest for it to be changed?

 > Making library functions suitable for use in persistent applications
 > amounts to a complete re-write. If you're interested in taking that
 > route, please let us know when you've finished.

	To put it short, yes, I feel that the rewrite is both possible
	and in the long run, inevitable, and no, I'm not in the rush to
	start it just now.

	An obvious approach to the problem may be to reimplement the
	non-returning functions on top of some (possibly newly
	introduced) functions that will return.

int
do_foo_and_do_not_return (ARGUMENTS...)
{
  int rv = do_foo_and_return (ARGUMENTS...);
  if (rv < 0) {
    /* . */
    G_fatal_error ("%s", CAUSE);
  }

  /* . */
  return rv;
}

	It seems that there's both G_find_cell () and G_find_cell2 () to
	overcome a different, though in some respect close issue.

	BTW, shouldn't G_find_cell () be marked as ``deprecated'' in
	`include/gisdefs.h' (if the compiler permits it), so that a
	warning will be issued for each its use?

 > [1] You can use G_set_error_routine() to install an error handler,
 > but the handler mustn't return (or else exit() will still get
 > called). The reason is that code which calls G_fatal_error() doesn't
 > expect it to return.

[...]

 > However, the handler must solve all problems itself. You can't just
 > push the responsibility onto the ~650 different files which call that
 > function.

	Obviously.



More information about the grass-dev mailing list