[GRASS-dev] set_fatal_error

Hi all,

in Vlib/error.c are defined fns Vect_get_fatal_error(),
Vect_set_fatal_error() which are currently used only in Vlib/open.c.

00049 static void
00050 fatal_error ( int ferror, char *errmsg )
00051 {
00052 switch ( ferror ) {
00053 case GV_FATAL_EXIT:
00054 G_fatal_error ( errmsg );
00055 break;
00056 case GV_FATAL_PRINT:
00057 G_warning ( errmsg );
00058 break;
00059 case GV_FATAL_RETURN:
00060 break;
00061 }
00062 }

Maybe it would be good to move them to gislib, to enable programmer
change behaviour of G_fatal_error() when really need, e.g. to avoid
crashing GUI (qgis or native wxpython-based).

?

Martin

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

Martin Landa wrote:

in Vlib/error.c are defined fns Vect_get_fatal_error(),
Vect_set_fatal_error() which are currently used only in Vlib/open.c.

00049 static void
00050 fatal_error ( int ferror, char *errmsg )
00051 {
00052 switch ( ferror ) {
00053 case GV_FATAL_EXIT:
00054 G_fatal_error ( errmsg );
00055 break;
00056 case GV_FATAL_PRINT:
00057 G_warning ( errmsg );
00058 break;
00059 case GV_FATAL_RETURN:
00060 break;
00061 }
00062 }

Maybe it would be good to move them to gislib, to enable programmer
change behaviour of G_fatal_error() when really need, e.g. to avoid
crashing GUI (qgis or native wxpython-based).

We already have G_set_error_routine(). However, a fundamental property
of G_fatal_error() is that it doesn't return (that's why the "fatal"
is part of the name).

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

Hi,

2008/1/14, Glynn Clements <glynn@gclements.plus.com>:

Martin:

> in Vlib/error.c are defined fns Vect_get_fatal_error(),
> Vect_set_fatal_error() which are currently used only in Vlib/open.c.
>
> 00049 static void
> 00050 fatal_error ( int ferror, char *errmsg )
> 00051 {
> 00052 switch ( ferror ) {
> 00053 case GV_FATAL_EXIT:
> 00054 G_fatal_error ( errmsg );
> 00055 break;
> 00056 case GV_FATAL_PRINT:
> 00057 G_warning ( errmsg );
> 00058 break;
> 00059 case GV_FATAL_RETURN:
> 00060 break;
> 00061 }
> 00062 }
>
> Maybe it would be good to move them to gislib, to enable programmer
> change behaviour of G_fatal_error() when really need, e.g. to avoid
> crashing GUI (qgis or native wxpython-based).

Glynn:

We already have G_set_error_routine(). However, a fundamental property
of G_fatal_error() is that it doesn't return (that's why the "fatal"
is part of the name).

right, but it doesn't avoid calling exit(EXIT_FAILURE) in G_fatal_error().

Calling G_fatal_error() causes crashing wxPython GUI e.g. during
digitization. I am not sure how to handle this, since G_fatal_error()
is widely used in Vector library. I guess this could be possible to
solve by using separate thread in Python code(?).

The second point, I guess it was the reason (crashing e.g. qgis) why
(probably by Radim) Vect_*_fatal_error() were added to Vect library.
But it is used only in open.c and legal_vname.c as far as i know.

Avoiding calling exit() in G_fatal_error() in the given case would
make sense, maybe.

Martin

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

Hi,

2008/1/14, Martin Landa <landa.martin@gmail.com>:

Glynn:
> We already have G_set_error_routine(). However, a fundamental property
> of G_fatal_error() is that it doesn't return (that's why the "fatal"
> is part of the name).

right, but it doesn't avoid calling exit(EXIT_FAILURE) in G_fatal_error().

Calling G_fatal_error() causes crashing wxPython GUI e.g. during
digitization. I am not sure how to handle this, since G_fatal_error()
is widely used in Vector library. I guess this could be possible to
solve by using separate thread in Python code(?).

The second point, I guess it was the reason (crashing e.g. qgis) why
(probably by Radim) Vect_*_fatal_error() were added to Vect library.
But it is used only in open.c and legal_vname.c as far as i know.

Avoiding calling exit() in G_fatal_error() in the given case would
make sense, maybe.

I meant something like:

int G_fatal_error (const char *msg,...)
{
    char buffer[2000]; /* No novels to the error logs, OK? */
    va_list ap;

    va_start(ap,msg);
    vsprintf(buffer,msg,ap);
    va_end(ap);

    print_error (buffer,ERR);

    if (ferror == G_FATAL_RETURN) {
  return EXIT_FAILURE;
    }

    exit (EXIT_FAILURE);
}

Martin

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

Martin Landa wrote:

> > We already have G_set_error_routine(). However, a fundamental property
> > of G_fatal_error() is that it doesn't return (that's why the "fatal"
> > is part of the name).
>
> right, but it doesn't avoid calling exit(EXIT_FAILURE) in G_fatal_error().
>
> Calling G_fatal_error() causes crashing wxPython GUI e.g. during
> digitization. I am not sure how to handle this, since G_fatal_error()
> is widely used in Vector library. I guess this could be possible to
> solve by using separate thread in Python code(?).
>
> The second point, I guess it was the reason (crashing e.g. qgis) why
> (probably by Radim) Vect_*_fatal_error() were added to Vect library.
> But it is used only in open.c and legal_vname.c as far as i know.
>
> Avoiding calling exit() in G_fatal_error() in the given case would
> make sense, maybe.

I meant something like:

int G_fatal_error (const char *msg,...)
{
    char buffer[2000]; /* No novels to the error logs, OK? */
    va_list ap;

    va_start(ap,msg);
    vsprintf(buffer,msg,ap);
    va_end(ap);

    print_error (buffer,ERR);

    if (ferror == G_FATAL_RETURN) {
  return EXIT_FAILURE;
    }

Nope; G_fatal_error() must *never* return; that's the whole point.

    exit (EXIT_FAILURE);

Code which calls G_fatal_error() relies upon it not returning. If it
were to return, the caller would most likely just crash as a
consequence of whichever "error" it was using G_fatal_error() to
report.

E.g. if you have the following code:

  struct foo *p = get_foo();
  if (!p)
    G_fatal_error("...");
  if (p->foo)
    ...;

If G_fatal_error() returns, you will just get a segfault when the code
dereferences the pointer.

Code which calls G_fatal_error() isn't just expecting an error message
to be printed. More importantly, it expects execution of the remainder
of the function to be aborted.

Many GRASS functions are written (and used) on the assumption that
they cannot fail. They either succeed, or they never return (because
any errors result in a call to G_fatal_error(), which never returns).

The GRASS libraries were designed for use in one-shot utilities.
Making them useful for persistent applications is going to require a
substantial re-write. You can't just tweak G_fatal_error(); you have
to re-write everything which calls it to actually recover from errors
rather than just terminating the program as soon as anything goes
wrong.

Even if you take the approach QGIS uses and install an error handler
which longjmp()s out to the caller, you have to allow for the fact
that the data structures used by the GRASS libraries may be left in an
inconsistent state, so subsequent library calls may simply segfault.

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

Hi,

2008/1/16, Glynn Clements <glynn@gclements.plus.com>:

> > > We already have G_set_error_routine(). However, a fundamental property
> > > of G_fatal_error() is that it doesn't return (that's why the "fatal"
> > > is part of the name).
> >
> > right, but it doesn't avoid calling exit(EXIT_FAILURE) in G_fatal_error().
> >
> > Calling G_fatal_error() causes crashing wxPython GUI e.g. during
> > digitization. I am not sure how to handle this, since G_fatal_error()
> > is widely used in Vector library. I guess this could be possible to
> > solve by using separate thread in Python code(?).
> >
> > The second point, I guess it was the reason (crashing e.g. qgis) why
> > (probably by Radim) Vect_*_fatal_error() were added to Vect library.
> > But it is used only in open.c and legal_vname.c as far as i know.
> >
> > Avoiding calling exit() in G_fatal_error() in the given case would
> > make sense, maybe.
>
> I meant something like:
>
> int G_fatal_error (const char *msg,...)
> {
> char buffer[2000]; /* No novels to the error logs, OK? */
> va_list ap;
>
> va_start(ap,msg);
> vsprintf(buffer,msg,ap);
> va_end(ap);
>
> print_error (buffer,ERR);
>
> if (ferror == G_FATAL_RETURN) {
> return EXIT_FAILURE;
> }

Nope; G_fatal_error() must *never* return; that's the whole point.

> exit (EXIT_FAILURE);

Code which calls G_fatal_error() relies upon it not returning. If it
were to return, the caller would most likely just crash as a
consequence of whichever "error" it was using G_fatal_error() to
report.

E.g. if you have the following code:

        struct foo *p = get_foo();
        if (!p)
                G_fatal_error("...");
        if (p->foo)
                ...;

If G_fatal_error() returns, you will just get a segfault when the code
dereferences the pointer.

Code which calls G_fatal_error() isn't just expecting an error message
to be printed. More importantly, it expects execution of the remainder
of the function to be aborted.

Many GRASS functions are written (and used) on the assumption that
they cannot fail. They either succeed, or they never return (because
any errors result in a call to G_fatal_error(), which never returns).

The GRASS libraries were designed for use in one-shot utilities.
Making them useful for persistent applications is going to require a
substantial re-write. You can't just tweak G_fatal_error(); you have
to re-write everything which calls it to actually recover from errors
rather than just terminating the program as soon as anything goes
wrong.

Even if you take the approach QGIS uses and install an error handler
which longjmp()s out to the caller, you have to allow for the fact
that the data structures used by the GRASS libraries may be left in an
inconsistent state, so subsequent library calls may simply segfault.

well, thanks a lot for the clarification.

Regards, Martin

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *