Обсуждение: [PATCH] fix two shadow vars (src/backend/commands/sequence.c)
Hi,
src/backend/commands/sequence.c
Has two shadows (buf var), with two unnecessary variables declared.
For readability reasons, the declaration of variable names in the prototypes was also corrected.
regards,
Ranier Vilela
Вложения
On 2020-Jun-11, Ranier Vilela wrote: > Hi, > src/backend/commands/sequence.c > Has two shadows (buf var), with two unnecessary variables declared. These are not unnecessary -- removing them breaks translatability of those messages. If these were ssize_t you could use '%zd' (see commit ac4ef637ad2f) but I don't think you can in this case. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Em qui., 11 de jun. de 2020 às 17:19, Alvaro Herrera <alvherre@2ndquadrant.com> escreveu:
On 2020-Jun-11, Ranier Vilela wrote:
> Hi,
> src/backend/commands/sequence.c
> Has two shadows (buf var), with two unnecessary variables declared.
These are not unnecessary -- removing them breaks translatability of
those messages. If these were ssize_t you could use '%zd' (see commit
ac4ef637ad2f) but I don't think you can in this case.
Hi Alvaro, thanks for reply.
File backend\utils\sort\tuplesort.c:
elog(LOG, "worker %d using " INT64_FORMAT " KB of memory for read buffers among %d input tapes",
elog(LOG, "worker %d using " INT64_FORMAT " KB of memory for read buffers among %d input tapes",
File backend\storage\ipc\shm_toc.c:
elog(ERROR, "could not find key " UINT64_FORMAT " in shm TOC at %p",
elog(ERROR, "could not find key " UINT64_FORMAT " in shm TOC at %p",
File backend\storage\large_object\inv_api.c:
* use errmsg_internal here because we don't want to expose INT64_FORMAT
errmsg_internal("invalid large object seek target: " INT64_FORMAT,
* use errmsg_internal here because we don't want to expose INT64_FORMAT
errmsg_internal("invalid large object seek target: " INT64_FORMAT,
elog and errmsg_internal, permits use as proposed by the patch,
does it mean that errmsg, does not allow and does not do the same job as snprintf?
regards,
Ranier Vilela
Ranier Vilela <ranier.vf@gmail.com> writes:
> elog and errmsg_internal, permits use as proposed by the patch,
> does it mean that errmsg, does not allow and does not do the same job as
> snprintf?
Yes. errmsg() strings are captured for translation. If they contain
platform-dependent substrings, that's a problem, because only one variant
will get captured. And INT64_FORMAT is platform-dependent.
We have of late decided that it's safe to use %lld (or %llu) to format
int64s everywhere, but you then have to cast the printf argument to
match that explicitly. See commit 6a1cd8b92 for precedent.
regards, tom lane
Em qui., 11 de jun. de 2020 às 19:54, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Ranier Vilela <ranier.vf@gmail.com> writes:
> elog and errmsg_internal, permits use as proposed by the patch,
> does it mean that errmsg, does not allow and does not do the same job as
> snprintf?
Yes. errmsg() strings are captured for translation. If they contain
platform-dependent substrings, that's a problem, because only one variant
will get captured. And INT64_FORMAT is platform-dependent.
We have of late decided that it's safe to use %lld (or %llu) to format
int64s everywhere, but you then have to cast the printf argument to
match that explicitly. See commit 6a1cd8b92 for precedent.
Hi Tom, thank you for the detailed explanation.
I see commit 6a1cd8b92, and I think which is the same case with basebackup.c (total_checksum_failures),
maxv and minv, are int64 (INT64_FORMAT).
%lld -> (long long int) maxv
%lld -> (long long int) minv
Attached new patch, with fixes from commit 6a1cd8b92.
regards,
Ranier Vilela
regards, tom lane