return correct error code from pgtls_init

Started by Zhihong Yualmost 5 years ago4 messageshackers
Jump to latest
#1Zhihong Yu
zyu@yugabyte.com

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

Attachments:

tls-init-error-handling.patchapplication/octet-stream; name=tls-init-error-handling.patchDownload+5-4
#2Michael Paquier
michael@paquier.xyz
In reply to: Zhihong Yu (#1)
Re: return correct error code from pgtls_init

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

#3Zhihong Yu
zyu@yugabyte.com
In reply to: Michael Paquier (#2)
Re: return correct error code from pgtls_init

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

#4Michael Paquier
michael@paquier.xyz
In reply to: Zhihong Yu (#3)
Re: return correct error code from pgtls_init

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