On 1/6/24 15:10, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> The only code specific comments were Tom's above, which have been
>> addressed. If there are no serious objections I plan to commit this
>> relatively soon.
>
> I had not actually read this patchset before, but now I have, and
> I have a few minor suggestions:
Many thanks!
> * The API comment for PQchangePassword should specify that encryption
> is done according to the server's password_encryption setting, and
> probably the SGML docs should too. You shouldn't have to read the
> code to discover that.
Check
> * I don't especially care for the useless initializations of
> encrypted_password, fmtpw, and fmtuser. In all those cases the
> initial NULL is immediately replaced by a valid value. Probably
> the compiler will figure out that the initializations are useless,
> but human readers have to do so as well. Moreover, I think this
> style is more bug-prone not less so, because if you ever change
> the logic in a way that causes some code paths to fail to set
> the variables, you won't get use-of-possibly-uninitialized-value
> warnings from the compiler.
>
> * Perhaps move the declaration of "buf" to the inner block where
> it's actually used?
Makes sense -- fixed
> * This could be shortened to just "return res":
>
> + if (!res)
> + return NULL;
> + else
> + return res;
Heh, apparently I needed more coffee at this point :-)
> * I'd make the SGML documentation a bit more explicit about the
> return value, say
>
> + Returns a <structname>PGresult</structname> pointer representing
> + the result of the <literal>ALTER USER</literal> command, or
> + a null pointer if the routine failed before issuing any command.
Fixed.
I also ran pgindent. I was kind of surprised/unhappy when it made me
change this (which aligned the two var names):
8<------------
<tab><tab><tab><tab>PQExpBufferData<tab>buf;
<tab><tab><tab><tab>PGresult<tab><sp><sp><sp>*res;
8<------------
to this (which leaves the var names unaligned):
8<------------
<tab><tab><tab><tab>PQExpBufferData<sp>buf;
<tab><tab><tab><tab>PGresult<sp><sp><sp>*res;
8<------------
Anyway, the resulting adjustments attached.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com