Re: Improve error handling of HMAC computations and SCRAM

Sergey Shinderuk <s.shinderuk@postgrespro.ru>

From: Sergey Shinderuk <s.shinderuk@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-01-11T07:50:50Z
Lists: pgsql-hackers
Hi,

On 11.01.2022 07:56, Michael Paquier wrote:
 > Thoughts?

A few comments after a quick glance...

+ * Returns a static string providing errors about an error that happened

"errors about an error" looks odd.


+static const char *
+SSLerrmessage(unsigned long ecode)
+{
+	if (ecode == 0)
+		return NULL;
+
+	/*
+	 * This may return NULL, but we would fall back to a default error path if
+	 * that were the case.
+	 */
+	return ERR_reason_error_string(ecode);
+}

We already have SSLerrmessage elsewhere and it's documented to never 
return NULL.  I find that confusing.

If I have two distinct pg_hmac_ctx's, are their errreason's idependent 
from one another or do they really point to the same static buffer?


Regards,

-- 
Sergey Shinderuk		https://postgrespro.com/



Commits

  1. Improve error handling of HMAC computations

  2. Fix incorrect comments in hmac.c and hmac_openssl.c

  3. Fix comment related to pg_cryptohash_error()