Hi,
this is just too weird to not mention it.
If you write a function for a complicated task, designing the user
interface well requires hard work. But you might think that not
much can go wrong when designing a function for a trivial task.
Guess again...
Here is my new candidate for the worstly-designed trivial API:
SSL_CIPHER_description(3).
To start with, you have the choice of providing your own return
buffer to the function or letting it malloc(3) one for you.
That's certainly not making the design, the documentation, or the
usage easier, but oh well, there is more than one function providing
that dubious choice. But hold on.
If you provide your own buffer, even if it is large enough, the
function can still fail from ENOMEM (sic!). In that case, the
content of your buffer is not changed - in particular, if it was
uninitialized before, it may not be NUL-terminated afterwards,
inviting subsequent read buffer overruns.
On the other hand, if you do not provide your own buffer and memory
allocation fails, the function returns a pointer to a static string,
so you have no choice but to use strcmp(3) to detect the error
condition - even if you only want to print and then discard the
result, because you must free(3) the returned string on success,
but of course you must *not* free the returned error string...
Apparently, the OpenSSL folks noticed that all this is not quite ideal.
So they changed the semantics of the interface for 1.1.0, without
renaming it. So in 1.1.0 and later, it does return NULL on failure,
as it should always have.
Consequently, here is the SIMPLEST possible, correct snippet for
using this interface in application code with a user-provided buffer:
desc = SSL_CIPHER_description(cipher, buf, sizeof(buf));
if (desc == NULL) /* Required because we might use 1.1.0. */
warnx("SSL_CIPHER_description: buffer too small");
else if (strcmp(desc, "OPENSSL_malloc Error") == 0)
warn("SSL_CIPHER_description");
else if (strcmp(desc, "Buffer too small") == 0)
printf("%s (truncated)\n", buf);
else
puts(buf);
You might hope that using auto-alloc could be simpler, but not
much - here is the SIMPLEST possible, correct code for that case:
desc = SSL_CIPHER_description(cipher, NULL, 0);
if (desc == NULL || strcmp(desc, "OPENSSL_malloc Error") == 0)
warn("SSL_CIPHER_description");
/* We must not call free(3) here. */
else {
puts(desc);
free(desc);
}
Any lesson from this mess? Well, you can always fix bugs in your
implementation, but PLEASE get the freaking API right the first time!
Now, should i add these two snippets as EXAMPLES to the manual
page? We can't really hope that any programmer will get it right
without seeing the examples, or can we?
Sigh,
Ingo
