[PATCH] fix two shadow vars (src/backend/commands/sequence.c)
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t42507psql -h localhost -U postgresBuilt from patchset v5 (message #5), July 28, 2026 at 02:31 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t42507_5 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t42507_5 && git checkout t42507_5Patchset v5 (message #5) is on t42507_5
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
Attachments:
fix_shadows_buf_var.patchapplication/octet-stream; name=fix_shadows_buf_var.patchDownload+7-13
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",
File backend\storage\ipc\shm_toc.c:
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,
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
Show quoted text
regards, tom lane