Re: [PATCH] Detail all X509_V_ERRs

On 03/24/2011 04:48 AM, Tsantilas Christos wrote:

> This simple patch provide %D details for all SSL errors documented at
> http://www.openssl.org/docs/apps/verify.html
>
> This patch also adds a std::map based structure to optimize the ssl
> error description retrieval.

> + const char *err = getErrorDescr(error_no);
> + if (!err)
> + return "[Not available]";
> + else
> + return err;
> +

Consider a shorter/safer version:

if (const char *err = getErrorDescr(error_no))
    return err;
return "[Not available]";

> +//Use std::map to optimize search
> +std::map<Ssl::ssl_error_t, SslErrorDetailEntry *> TheSslDetailMap;
...
> + std::map<Ssl::ssl_error_t, SslErrorDetailEntry *>::iterator it;
> + for (it=TheSslDetailMap.begin(); it != TheSslDetailMap.end(); ++it) {
> + if (strcmp(name, it->second->name) == 0)
> + return it->second->value;
> }

Consider a more polished and slightly safer version:

> typedef std::map<Ssl::ssl_error_t, SslErrorDetailEntry *> SslErrorDetails;
> static SslErrorDetails TheSslDetailMap;
> ...
> typedef SslErrorDetails::const_iterator SEDCI;
> for (SEDCI i = TheSslDetailMap.begin(); i != TheSslDetailMap.end(); ++i) {
> ...

The SslErrorDetails type may be used here as well (also correcting for
const-ness):

> + const SslErrorDetails::const_iterator it = TheSslDetailMap.find(value);
> + if (it != TheSslDetailMap.end())

I would also rename TheSslDetailMap to TheSslDetails because the code
does not really care about the exact index structure. For example, when
hash_map becomes easily portable, we can start using that instead.

Thank you,

Alex.
Received on Thu Mar 24 2011 - 16:09:17 MDT