Thread: return correct error code from pgtls_init
Hi,
It seems error code checking in pgtls_init() should follow the same convention as PG codebase adopts - i.e. the non-zero error code should be returned (instead of hard coded -1).
Please see the attached patch.
Thanks
Attachment
On Tue, Jun 01, 2021 at 10:32:59AM -0700, Zhihong Yu wrote: > It seems error code checking in pgtls_init() should follow the same > convention as PG codebase adopts - i.e. the non-zero error code should be > returned (instead of hard coded -1). > > Please see the attached patch. I don't see the point of changing this. First, other areas of fe-secure-openssl.c use a harcoded value of -1 as error codes, so the current style is more consistent. Second, if we were to change that, why are you not changing one call of pthread_mutex_lock()? -- Michael
Attachment
On Tue, Jun 1, 2021 at 6:14 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Jun 01, 2021 at 10:32:59AM -0700, Zhihong Yu wrote:
> It seems error code checking in pgtls_init() should follow the same
> convention as PG codebase adopts - i.e. the non-zero error code should be
> returned (instead of hard coded -1).
>
> Please see the attached patch.
I don't see the point of changing this. First, other areas of
fe-secure-openssl.c use a harcoded value of -1 as error codes, so the
current style is more consistent. Second, if we were to change that,
why are you not changing one call of pthread_mutex_lock()?
--
Michael
Hi,
Looking at the -1 return, e.g.
pq_lockarray = malloc(sizeof(pthread_mutex_t) * CRYPTO_num_locks());
when pq_lockarray is NULL. We can return errno.
I didn't change call to pthread_mutex_lock() because PGTHREAD_ERROR() is used which aborts.
Cheers
On Tue, Jun 01, 2021 at 06:56:42PM -0700, Zhihong Yu wrote: > Looking at the -1 return, e.g. > > pq_lockarray = malloc(sizeof(pthread_mutex_t) * > CRYPTO_num_locks()); > > when pq_lockarray is NULL. We can return errno. > > I didn't change call to pthread_mutex_lock() because PGTHREAD_ERROR() is > used which aborts. I am not sure what you mean here, and there is nothing wrong with this code as far as I know, as we would let the caller of pgtls_init() know that something is wrong. -- Michael