[gnutls-devel] gnutls 3.5.5

[Stefan Bühler]

Hi Nikos,

On 10/22/2016 09:57 AM, Nikos Mavrogiannopoulos wrote:
> On Fri, 2016-10-21 at 20:08 +0200, Stefan Bühler wrote:
> 
>> I'd like to point out that gnutls_certificate_set_key* had an API
>> incompatible change: they now might return integers > 0 when
>> successful,
>> which breaks previously documented behaviour of always returning
>> GNUTLS_E_SUCCESS (i.e. 0).
>>
>> I already pointed this out when reviewing the change, see:
>>
>>     https://gitlab.com/gnutls/gnutls/merge_requests/65#note_15134877
>>
>> Nikos, how do you want to handle this in the future? Should I always
>> check for `>= GNUTLS_E_SUCCESS` in my code when testing for success?
>>
>> If so this should be properly reflected throughout the documentation.
> 
> Hi Stefan,
>  Thanks for bringing that up. I went through the documentation and the
> related changes, and they are indeed not sufficient for the
> documentation of such a change. I've tried to improve it at:
> https://gitlab.com/gnutls/gnutls/merge_requests/123
> (comments welcome)

Documenting the API changes seems ok, although I'm not sure everyone who
needs to know about will find it there :)

> My suggestion as you will see above in the merge request is for
> applications to use if (retval < 0) to check for errors rather than
> check for a specific success code. That cannot of course change past
> applications.
> 
> My expectation was that few software will check for equality with zero,
> mainly due to the examples which use the <0 pattern, and that even if
> they did, the fact that the first certificate index would be zero, will
> mitigate any issue (most applications load a single certificate).

I don't think I've ever read the generic Error handling section; I only
read the sections for single functions.  I think those should be adapted
as well, they simply don't read like you have to expect positive values
as success too:

    On success, GNUTLS_E_SUCCESS (0) is returned, otherwise an error
    code is returned.

Maybe something like this instead:

    On success, a value equal to or greater than GNUTLS_E_SUCCESS (0)
    is returned, otherwise a negative error code is returned.

Gonna be a large patch though :)

> Do you think this is going to cause issues? Most likely we can still
> revert the change by introducing a flag in
> gnutls_certificate_set_flags() which can enable the behavior of
> returning indexes, instead of returning them by default.

It did cause issues with lighttpd2; as there is no release and I fixed
it in git HEAD I don't see any remaining issues in this case.

I can't speak for other applications :)

regards,
Stefan