On 1/18/22 15:41, Tom Lane wrote:
David Steele <da...@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
