[openssl-project] Style question

[Matt Caswell]

On 12/02/18 16:07, Matt Caswell wrote:
> 
> 
> On 12/02/18 16:05, Short, Todd wrote:
>> My 2cents (since I can’t reply to the list), is that other functions
>> (e.g. most SSL and SSL_CTX functions) require a non-NULL object. I’m not
>> sure this is any different.
> 
> Good point. Although that then changes the question slightly to be can
> we assume that this function will never return an error (even though it
> has in the past)?

But then thinking about it more - I don't think we can remove this
check. Since we are currently checking this, removing it could cause a
binary compat issue if some application is relying on that check.

Matt


> 
> 
> (BTW you can reply to the list - its just moderated so you have to wait
> for someone to approve it.)
> 
> Matt
> 
>>
>> --
>> -Todd Short
>> // tshort at akamai.com <mailto:tshort at akamai.com>
>> // "One if by land, two if by sea, three if by the Internet."
>>
>>> On Feb 12, 2018, at 11:02 AM, Matt Caswell <matt at openssl.org
>>> <mailto:matt at openssl.org>> wrote:
>>>
>>> I've been looking at our use of EVP_MD_size() (prompted by Coverity).
>>>
>>> That function can return a -1 on error:
>>>
>>> int EVP_MD_size(const EVP_MD *md)
>>> {
>>>    if (!md) {
>>>        EVPerr(EVP_F_EVP_MD_SIZE, EVP_R_MESSAGE_DIGEST_IS_NULL);
>>>        return -1;
>>>    }
>>>    return md->md_size;
>>> }
>>>
>>>
>>> The only (current) possible error is that the passed digest is NULL.
>>> Otherwise it returns the size of the digest as you would expect.
>>>
>>> In some places we do things like this:
>>>
>>>        const EVP_MD *md = ssl_md(s->session->cipher->algorithm2);
>>>
>>>        if (md != NULL) {
>>>            /*
>>>             * Add the fixed PSK overhead, the identity length and the
>>> binder
>>>             * length.
>>>             */
>>>            hlen +=  PSK_PRE_BINDER_OVERHEAD + s->session->ext.ticklen
>>>                     + EVP_MD_size(md);
>>>        }
>>>
>>> So we have an explicit NULL check of the md before we call the function.
>>> Therefore there is no possible way that EVP_MD_size() can return
>>> anything except a success response.
>>>
>>> Are we entitled to assume that? Or should we always check the return
>>> value regardless? My instinct says we should in case we ever wanted to
>>> change the function in the future to return an error in some other
>>> circumstances.
>>>
>>> Just to make it more interesting our documentation does not mention the
>>> possibility of an error return at all.
>>>
>>> Matt
>>> _______________________________________________
>>> openssl-project mailing list
>>> openssl-project at openssl.org <mailto:openssl-project at openssl.org>
>>> https://mta.openssl.org/mailman/listinfo/openssl-project
>>
> _______________________________________________
> openssl-project mailing list
> openssl-project at openssl.org
> https://mta.openssl.org/mailman/listinfo/openssl-project
>