Thread: INT64_FORMAT in translatable strings
Hello.
I found the following lines in xlogprefetch.c.
> ereport(LOG,
> (errmsg("recovery finished prefetching at %X/%X; "
> "prefetch = " UINT64_FORMAT ", "
> "skip_hit = " UINT64_FORMAT ", "
...
It is found in ja.po as
"recovery finished prefetching at %X/%X; prefetch = "
. . . .
Anyway we can rely on %lld/%llu and we decided to use them in
translatable strings. So the attached fixes (AFAICS) all instances of
the macros in translatable strings.
# I just found 3286065651 did one instance of that so I excluded that
# from this patch.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 50873d196fea546a78c2c26f1963e172dc95c687 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Wed, 21 Apr 2021 19:50:43 +0900
Subject: [PATCH] Don't use INT64_FORMAT inside message strings, take 2
Use %lld/%llu and cast to (unsigned) long long int instead.
---
contrib/pg_prewarm/pg_prewarm.c | 8 ++++----
src/backend/access/transam/xlogprefetch.c | 23 +++++++++++------------
src/bin/pgbench/pgbench.c | 7 ++++---
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/contrib/pg_prewarm/pg_prewarm.c b/contrib/pg_prewarm/pg_prewarm.c
index a855452936..58dcb6c76c 100644
--- a/contrib/pg_prewarm/pg_prewarm.c
+++ b/contrib/pg_prewarm/pg_prewarm.c
@@ -126,8 +126,8 @@ pg_prewarm(PG_FUNCTION_ARGS)
if (first_block < 0 || first_block >= nblocks)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("starting block number must be between 0 and " INT64_FORMAT,
- nblocks - 1)));
+ errmsg("starting block number must be between 0 and %lld",
+ (long long int) (nblocks - 1))));
}
if (PG_ARGISNULL(4))
last_block = nblocks - 1;
@@ -137,8 +137,8 @@ pg_prewarm(PG_FUNCTION_ARGS)
if (last_block < 0 || last_block >= nblocks)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("ending block number must be between 0 and " INT64_FORMAT,
- nblocks - 1)));
+ errmsg("ending block number must be between 0 and %lld",
+ (long long int) (nblocks - 1))));
}
/* Now we're ready to do the real work. */
diff --git a/src/backend/access/transam/xlogprefetch.c b/src/backend/access/transam/xlogprefetch.c
index 9a6f17ca36..3d401bda34 100644
--- a/src/backend/access/transam/xlogprefetch.c
+++ b/src/backend/access/transam/xlogprefetch.c
@@ -358,20 +358,19 @@ XLogPrefetcherFree(XLogPrefetcher *prefetcher)
/* Log final statistics. */
ereport(LOG,
(errmsg("recovery finished prefetching at %X/%X; "
- "prefetch = " UINT64_FORMAT ", "
- "skip_hit = " UINT64_FORMAT ", "
- "skip_new = " UINT64_FORMAT ", "
- "skip_fpw = " UINT64_FORMAT ", "
- "skip_seq = " UINT64_FORMAT ", "
+ "prefetch = %llu, "
+ "skip_hit = %llu, "
+ "skip_new = %llu, "
+ "skip_fpw = %llu, "
+ "skip_seq = %llu, "
"avg_distance = %f, "
"avg_queue_depth = %f",
- (uint32) (prefetcher->reader->EndRecPtr << 32),
- (uint32) (prefetcher->reader->EndRecPtr),
- pg_atomic_read_u64(&SharedStats->prefetch),
- pg_atomic_read_u64(&SharedStats->skip_hit),
- pg_atomic_read_u64(&SharedStats->skip_new),
- pg_atomic_read_u64(&SharedStats->skip_fpw),
- pg_atomic_read_u64(&SharedStats->skip_seq),
+ LSN_FORMAT_ARGS(prefetcher->reader->EndRecPtr),
+ (unsigned long long int) pg_atomic_read_u64(&SharedStats->prefetch),
+ (unsigned long long int) pg_atomic_read_u64(&SharedStats->skip_hit),
+ (unsigned long long int) pg_atomic_read_u64(&SharedStats->skip_new),
+ (unsigned long long int) pg_atomic_read_u64(&SharedStats->skip_fpw),
+ (unsigned long long int) pg_atomic_read_u64(&SharedStats->skip_seq),
SharedStats->avg_distance,
SharedStats->avg_queue_depth)));
hash_destroy(prefetcher->filter_table);
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index da1d9ec535..07403abb58 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5359,8 +5359,8 @@ parseScriptWeight(const char *option, char **script)
}
if (wtmp > INT_MAX || wtmp < 0)
{
- pg_log_fatal("weight specification out of range (0 .. %u): " INT64_FORMAT,
- INT_MAX, (int64) wtmp);
+ pg_log_fatal("weight specification out of range (0 .. %u): %lld",
+ INT_MAX, (long long int) wtmp);
exit(1);
}
weight = wtmp;
@@ -5680,7 +5680,8 @@ set_random_seed(const char *seed)
}
if (seed != NULL)
- pg_log_info("setting random seed to " UINT64_FORMAT, iseed);
+ pg_log_info("setting random seed to %llu",
+ (unsigned long long int) iseed);
random_seed = iseed;
/* Fill base_random_sequence with low-order bits of seed */
--
2.27.0
On Wed, Apr 21, 2021 at 08:00:00PM +0900, Kyotaro Horiguchi wrote: > Anyway we can rely on %lld/%llu and we decided to use them in > translatable strings. So the attached fixes (AFAICS) all instances of > the macros in translatable strings. Indeed, good catch. Thanks. > # I just found 3286065651 did one instance of that so I excluded that > # from this patch. May I ask why you are using "unsigned long long int" rather uint64? What you are proposing is more consistent with what's done in the signed case like 3286065, so no objections from me, but I was just wondering. Personally, I think that I would just use "unsigned long long", like in xlogreader.c or pg_controldata.c to take two examples. -- Michael
Attachment
Julien Rouhaud <rjuju123@gmail.com> writes: > On Thu, Apr 22, 2021 at 07:49:23PM +0900, Michael Paquier wrote: >> May I ask why you are using "unsigned long long int" rather uint64? > My understanding is that it's the project standard. See e.g. > https://www.postgresql.org/message-id/1730584.1617836485@sss.pgh.pa.us Indeed, using %lld, %llu, etc with a matching cast to "long long" or "unsigned long long" is the approved way. Don't use [u]int64 because that does not necessarily match these format specs. It's probably physically compatible, but that won't stop pickier compilers from nagging about a format mismatch. But what I thought Michael was griping about is the use of "int", which is a noise word here. Either "long long int" or "long long" will work, but I think we've preferred the latter because shorter. regards, tom lane
At Thu, 22 Apr 2021 09:29:46 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in > Julien Rouhaud <rjuju123@gmail.com> writes: > > On Thu, Apr 22, 2021 at 07:49:23PM +0900, Michael Paquier wrote: > >> May I ask why you are using "unsigned long long int" rather uint64? > > > My understanding is that it's the project standard. See e.g. > > https://www.postgresql.org/message-id/1730584.1617836485@sss.pgh.pa.us > > Indeed, using %lld, %llu, etc with a matching cast to "long long" or > "unsigned long long" is the approved way. Don't use [u]int64 because > that does not necessarily match these format specs. It's probably > physically compatible, but that won't stop pickier compilers from > nagging about a format mismatch. > > But what I thought Michael was griping about is the use of "int", > which is a noise word here. Either "long long int" or "long long" > will work, but I think we've preferred the latter because shorter. Yeah, there's no reason for the "int" other than just following the immediate preceding commit 3286065651. I also prefer the shorter notations. Attached. regards. -- Kyotaro Horiguchi NTT Open Source Software Center From 8a4dddaaca59339ea69019c4d1d9c8a8481a7768 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Wed, 21 Apr 2021 19:50:43 +0900 Subject: [PATCH v2] Don't use INT64_FORMAT inside message strings, take 2 Use %lld/%llu instead of (U)INT64_FORMAT. Nowadays our standard way of printing int64 uint64 values at least in translatable strings is using the conversion specifications "%lld" and "%llu" and explicitly casting the parameters into "long long" and "unsigned long long" respectively. Along with that, we prefer to use "long" than "long int". See commit 595a0eab7f425e3484639fae1f7e221fe9c2651a. --- contrib/pg_prewarm/pg_prewarm.c | 8 ++++---- src/backend/access/transam/xlogprefetch.c | 23 +++++++++++------------ src/bin/pgbench/pgbench.c | 6 +++--- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/contrib/pg_prewarm/pg_prewarm.c b/contrib/pg_prewarm/pg_prewarm.c index a855452936..48d0132a0d 100644 --- a/contrib/pg_prewarm/pg_prewarm.c +++ b/contrib/pg_prewarm/pg_prewarm.c @@ -126,8 +126,8 @@ pg_prewarm(PG_FUNCTION_ARGS) if (first_block < 0 || first_block >= nblocks) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("starting block number must be between 0 and " INT64_FORMAT, - nblocks - 1))); + errmsg("starting block number must be between 0 and %lld", + (long long) (nblocks - 1)))); } if (PG_ARGISNULL(4)) last_block = nblocks - 1; @@ -137,8 +137,8 @@ pg_prewarm(PG_FUNCTION_ARGS) if (last_block < 0 || last_block >= nblocks) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("ending block number must be between 0 and " INT64_FORMAT, - nblocks - 1))); + errmsg("ending block number must be between 0 and %lld", + (long long) (nblocks - 1)))); } /* Now we're ready to do the real work. */ diff --git a/src/backend/access/transam/xlogprefetch.c b/src/backend/access/transam/xlogprefetch.c index 9a6f17ca36..ae4585232b 100644 --- a/src/backend/access/transam/xlogprefetch.c +++ b/src/backend/access/transam/xlogprefetch.c @@ -358,20 +358,19 @@ XLogPrefetcherFree(XLogPrefetcher *prefetcher) /* Log final statistics. */ ereport(LOG, (errmsg("recovery finished prefetching at %X/%X; " - "prefetch = " UINT64_FORMAT ", " - "skip_hit = " UINT64_FORMAT ", " - "skip_new = " UINT64_FORMAT ", " - "skip_fpw = " UINT64_FORMAT ", " - "skip_seq = " UINT64_FORMAT ", " + "prefetch = %llu, " + "skip_hit = %llu, " + "skip_new = %llu, " + "skip_fpw = %llu, " + "skip_seq = %llu, " "avg_distance = %f, " "avg_queue_depth = %f", - (uint32) (prefetcher->reader->EndRecPtr << 32), - (uint32) (prefetcher->reader->EndRecPtr), - pg_atomic_read_u64(&SharedStats->prefetch), - pg_atomic_read_u64(&SharedStats->skip_hit), - pg_atomic_read_u64(&SharedStats->skip_new), - pg_atomic_read_u64(&SharedStats->skip_fpw), - pg_atomic_read_u64(&SharedStats->skip_seq), + LSN_FORMAT_ARGS(prefetcher->reader->EndRecPtr), + (unsigned long long) pg_atomic_read_u64(&SharedStats->prefetch), + (unsigned long long) pg_atomic_read_u64(&SharedStats->skip_hit), + (unsigned long long) pg_atomic_read_u64(&SharedStats->skip_new), + (unsigned long long) pg_atomic_read_u64(&SharedStats->skip_fpw), + (unsigned long long) pg_atomic_read_u64(&SharedStats->skip_seq), SharedStats->avg_distance, SharedStats->avg_queue_depth))); hash_destroy(prefetcher->filter_table); diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index da1d9ec535..0827665997 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -5359,8 +5359,8 @@ parseScriptWeight(const char *option, char **script) } if (wtmp > INT_MAX || wtmp < 0) { - pg_log_fatal("weight specification out of range (0 .. %u): " INT64_FORMAT, - INT_MAX, (int64) wtmp); + pg_log_fatal("weight specification out of range (0 .. %u): %lld", + INT_MAX, (long long) wtmp); exit(1); } weight = wtmp; @@ -5680,7 +5680,7 @@ set_random_seed(const char *seed) } if (seed != NULL) - pg_log_info("setting random seed to " UINT64_FORMAT, iseed); + pg_log_info("setting random seed to %llu", (unsigned long long) iseed); random_seed = iseed; /* Fill base_random_sequence with low-order bits of seed */ -- 2.27.0
At Fri, 23 Apr 2021 13:26:09 +0900, Michael Paquier <michael@paquier.xyz> wrote in > On Fri, Apr 23, 2021 at 09:43:09AM +0900, Kyotaro Horiguchi wrote: > > At Thu, 22 Apr 2021 09:29:46 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in > >> But what I thought Michael was griping about is the use of "int", > >> which is a noise word here. Either "long long int" or "long long" > >> will work, but I think we've preferred the latter because shorter. > > Yep, that's what I meant. Sorry for the confusion. > > > Yeah, there's no reason for the "int" other than just following the > > immediate preceding commit 3286065651. I also prefer the shorter > > notations. Attached. > > Note that 3286065 only worked on signed integers. Yes. it uses redundant "int" for "long". > > - (uint32) (prefetcher->reader->EndRecPtr << 32), > > - (uint32) (prefetcher->reader->EndRecPtr), > > [..] > > + LSN_FORMAT_ARGS(prefetcher->reader->EndRecPtr), > > Good catch here. LSN_FORMAT_ARGS() exists to prevent such errors. > > And applied. Thanks! Thanks! -- Kyotaro Horiguchi NTT Open Source Software Center