Fix misuse use of pg_b64_encode function (contrib/postgres_fdw/connection.c)

On Wed, Jan 15, 2025 at 10:12:51PM -0300, Ranier Vilela wrote:

Fix by checking the return and reporting a message to the user,
in case of failure.

You are right that it is inconsistent to not check the result returned
by these two calls of pg_b64_encode(), as introduced in 761c79508e7f.
Peter?
--
Michael

On 16.01.25 02:12, Ranier Vilela wrote:

Per Coverity.

CID 1590024:    (CHECKED_RETURN)
Calling "pg_b64_encode" without checking return value (as is done
elsewhere 8 out of 10 times).

The function *pg_b64_encode* has in the comments:
[0]  "and -1 in the event of an error"

So, the function can fail.
All other calls check the return, In this case it could not be different.

Fix by checking the return and reporting a message to the user,
in case of failure.

Thanks, fixed. (I changed the ereports to elogs, which is how other
call sites do it.)

I also fixed a related problem in the pg_b64_decode() calls in libpq.

Maybe we could put a pg_nodiscard attribute on pg_b64_encode() and
pg_b64_decode()?

[0] I think the most correct would be *or* not *and* word?

I think both are ok here.

On Thu, Jan 16, 2025 at 09:07:45AM +0100, Peter Eisentraut wrote:

Maybe we could put a pg_nodiscard attribute on pg_b64_encode() and
pg_b64_decode()?

Sounds like a good idea to me. +1.
--
Michael

Em qui., 16 de jan. de 2025 às 05:07, Peter Eisentraut <peter@eisentraut.org>
escreveu:

On 16.01.25 02:12, Ranier Vilela wrote:

Per Coverity.

CID 1590024: (CHECKED_RETURN)
Calling "pg_b64_encode" without checking return value (as is done
elsewhere 8 out of 10 times).

The function *pg_b64_encode* has in the comments:
[0] "and -1 in the event of an error"

So, the function can fail.
All other calls check the return, In this case it could not be different.

Fix by checking the return and reporting a message to the user,
in case of failure.

Thanks, fixed. (I changed the ereports to elogs, which is how other
call sites do it.)

Thank you.

I also fixed a related problem in the pg_b64_decode() calls in libpq.

Maybe we could put a pg_nodiscard attribute on pg_b64_encode() and
pg_b64_decode()?

+1

[0] I think the most correct would be *or* not *and* word?

I think both are ok here.

Ok.

best regards,
Ranier Vilela

On 16.01.25 11:23, Ranier Vilela wrote:

Em qui., 16 de jan. de 2025 às 05:07, Peter Eisentraut
<peter@eisentraut.org <mailto:peter@eisentraut.org>> escreveu:

On 16.01.25 02:12, Ranier Vilela wrote:

Per Coverity.

CID 1590024:    (CHECKED_RETURN)
Calling "pg_b64_encode" without checking return value (as is done
elsewhere 8 out of 10 times).

The function *pg_b64_encode* has in the comments:
[0]  "and -1 in the event of an error"

So, the function can fail.
All other calls check the return, In this case it could not be

different.

Fix by checking the return and reporting a message to the user,
in case of failure.

Thanks, fixed.  (I changed the ereports to elogs, which is how other
call sites do it.)

Thank you.

I also fixed a related problem in the pg_b64_decode() calls in libpq.

Maybe we could put a pg_nodiscard attribute on pg_b64_encode() and
pg_b64_decode()?

+1

done

Em sex., 17 de jan. de 2025 às 05:11, Peter Eisentraut <peter@eisentraut.org>
escreveu:

On 16.01.25 11:23, Ranier Vilela wrote:

Em qui., 16 de jan. de 2025 às 05:07, Peter Eisentraut
<peter@eisentraut.org <mailto:peter@eisentraut.org>> escreveu:

On 16.01.25 02:12, Ranier Vilela wrote:

Per Coverity.

CID 1590024: (CHECKED_RETURN)
Calling "pg_b64_encode" without checking return value (as is done
elsewhere 8 out of 10 times).

The function *pg_b64_encode* has in the comments:
[0] "and -1 in the event of an error"

So, the function can fail.
All other calls check the return, In this case it could not be

different.

Fix by checking the return and reporting a message to the user,
in case of failure.

Thanks, fixed. (I changed the ereports to elogs, which is how other
call sites do it.)

Thank you.

I also fixed a related problem in the pg_b64_decode() calls in libpq.

Maybe we could put a pg_nodiscard attribute on pg_b64_encode() and
pg_b64_decode()?

+1

done

Thanks Peter.

best regards,
Ranier Vilela