Re: Allow root ownership of client certificate key

On 1/18/22 15:41, Tom Lane wrote:
> David Steele <david@pgmasters.net> writes:
> 
> I took a quick look at this and agree with the proposed behavior
> change, but also with your self-criticisms:
> 
>> We may want to do the same on the server side to make the code blocks
>> look more similar.
>>
>> Also, on the server side the S_ISREG() check gets its own error and that
>> might be a good idea on the client side as well. As it is, the error
>> message on the client is going to be pretty confusing in this case.
> 
> Particularly, I think the S_ISREG check should happen before any
> ownership/permissions checks; it just seems saner that way.

I was worried about doing too much refactoring in this commit since I 
have hopes and dreams of it being back-patched. But I'll go ahead and do 
that and if any part of this can be back-patched we'll consider that 
separately.

> The only other nitpick I have is that I'd make the cross-references be
> to the two file names, ie like "Note that similar checks are performed
> in fe-secure-openssl.c ..."  References to the specific functions seem
> likely to bit-rot in the face of future code rearrangements.
> I suppose filename references could become obsolete too, but it
> seems less likely.

It's true that functions are more likely to be renamed, but when I 
rename a function I then search for all the places where it is used so I 
can update them. If the function name appears in a comment that gets 
updated as well.

If you would still prefer filenames I have no strong argument against 
that, just wanted to explain my logic.

Regards,
-David