Re: [Bug-gsl] Erroneous use of GSL_ERROR_NULL instead of GSL_ERROR macro

[Matthias Sitte]

On 7/10/15 2:43 AM, Ofenloch, Oliver wrote:
> Wouldn't it be enough to simply change the macro to
>
> #define GSL_ERROR_NULL(reason, gsl_errno) GSL_ERROR_VAL(reason, gsl_errno, 1)
>
> With that modification, the macro would expand to
>
> do { \
>        gsl_error ("gsl_odeiv2_system dimension must be a positive integer",
"/home/ofenloch/teben2/gsl/ode-initval2/driver.c", 61, GSL_EINVAL) ; \
>        return 1 ; \
>        } while (0)
>
> Shouldn't that do the trick?

I don't think that will work.

AFAIK, GSL_ERROR_NULL is supposed to be used in functions which return pointers
and it should work like this: trigger gsl_error() with the given error message
and error code (which often, but not necessarily is equal to GSL_ENOMEM) so that
an error message is printed, and then return a NULL pointer (for example,
because malloc() failed).

Now, gsl_error() itself calls the registered error handler -- the default GSL
error handler prints the message and then aborts, so you'll actually never get
back to the caller. This is fine in development programs, but in production code
you often want to recover instead of abort. This can be done by checking the
return value of the function which is an invalid NULL pointer, and then you can
decide what to do.

In the current version, GSL_ERROR_NULL is used erroneously in functions that
return an integer status code. As it happens, NULL and GSL_SUCCESS both evaluate
to 0, so when GSL_ERROR_NULL is used instead of GSL_ERROR, you'll see
GSL_SUCCESS instead of, e.g., GSL_EINVAL. There's no way in a production code
which doesn't trigger abort() on error to detect that.

Regards,

  Matthias