On 2022-Jul-30, Dilip Kumar wrote: > On Thu, Jul 28, 2022 at 9:29 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> > wrote:
> > Please don't do this; rather use %llu and cast to (long long). > > Otherwise the string becomes mangled for translation. > > Okay, actually I did not understand the clear logic of when to use > %llu and to use (U)INT64_FORMAT. They are both used for 64-bit > integers. So do you think it is fine to replace all INT64_FORMAT in > my patch with %llu? The point here is that there are two users of the source code: one is the compiler, and the other is gettext, which extracts the string for the translation catalog. The compiler is OK with UINT64_FORMAT, of course (because the preprocessor deals with it). But gettext is quite stupid and doesn't understand that UINT64_FORMAT expands to some specifier, so it truncates the string at the double quote sign just before; in other words, it just doesn't work. So whenever you have a string that ends up in a translation catalog, you must not use UINT64_FORMAT or any other preprocessor macro; it has to be a straight specifier in the format string. We have found that the most convenient notation is to use %llu in the string and cast the argument to (unsigned long long), so our convention is to use that. For strings that do not end up in a translation catalog, there's no reason to use %llu-and-cast; UINT64_FORMAT is okay. > > > @@ -2378,7 +2378,7 @@ verifyBackupPageConsistency(XLogReaderState *record) > > > if (memcmp(replay_image_masked, primary_image_masked, > > > BLCKSZ) != 0) > > > { > > > elog(FATAL, > > > - "inconsistent page found, rel %u/%u/%u, > > > forknum %u, blkno %u", > > > + "inconsistent page found, rel %u/%u/" > > > INT64_FORMAT ", forknum %u, blkno %u", > > > rlocator.spcOid, rlocator.dbOid, > > > rlocator.relNumber, > > > forknum, blkno); > > > > Should this one be an ereport, and thus you do need to change it to that > > and handle it like that? > > Okay, so you mean irrespective of this patch should this be converted > to ereport? Yes, I think this should be an ereport with errcode(ERRCODE_DATA_CORRUPTED). -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/