Re: VALIDATE_PARAMETERS macro

[Simon Josefsson]

Jeffrey Walton <address@hidden> writes:

> Hi All,
>
> I'd like to introduce a VALIDATE_PARAMETERS macro. The macro would
> guard full parameter validation in library functions.
>
> I think full parameter validation would greatly enhance the robustness
> of the library by hardening the library from user land errors and
> mis-use. For those who are interested in performance ("the user needs
> to RTFM" philosophy), the macro can remain undefined to retain
> existing behavior.
>
> In addition, the asserts (or gnutls_assert) will aide in finding the
> point of first failure quickly, which frees developers up to do other
> things. (VALIDATE_PARAMETERS and asserts are tightly coupled in
> well-instrumented code). A proper assert strategy would include:
> Release, off; Debug, on; and Test, off.
>
> Below is a sample of existing and augmented code.
>
> Could anyone help with comments?

I don't think a VALIDATE_PARAMETERS symbol is necessary (actually I
consider any CPP #if's a nuisance from a code quality point of view: it
makes it difficult to test all code paths).  Instead, just add the NULL
checks in the code directly.  This is done in many places already.

> int
> gnutls_dh_params_import_raw (gnutls_dh_params_t dh_params,
>                            const gnutls_datum_t * prime,
>                            const gnutls_datum_t * generator)
> {
>   bigint_t tmp_prime, tmp_g;
>   size_t size;
>
> #if defined VALIDATE_PARAMETERS
>   if (dh_params == NULL || dh_params->params[0] == NULL
>       || dh_params->params[1] == NULL)
>     {
>       gnutls_assert ();
>       return GNUTLS_E_INVALID_REQUEST;
>     }
...

I'm not certain this is a good example: _gnutls_mpi_scan_nz appears to
check whether input variables are NULL or not.  So these additional
checks are redundant.

/Simon