Thank you for looking this.
At Tue, 19 Mar 2024 10:50:23 +0100, Peter Eisentraut <pe...@eisentraut.org>
wrote in
> On 15.03.24 08:20, Kyotaro Horiguchi wrote:
> > diff --git a/src/backend/access/transam/twophase.c
> > b/src/backend/access/transam/twophase.c
> > @@ -1369,8 +1369,8 @@ ReadTwoPhaseFile(TransactionId xid, bool
> > missing_ok)
> > errmsg("could not read file \"%s\":
> > %m", path)));
> > else
> > ereport(ERROR,
> > - (errmsg("could not read file \"%s\":
> > - read %d of %lld",
> > - path, r, (long long
> > - int) stat.st_size)));
> > + (errmsg("could not read file \"%s\": read %zd of %zu",
> > + path, r, (Size) stat.st_size)));
> > }
> > pgstat_report_wait_end();
>
> This might be worse, because stat.st_size is of type off_t, which
> could be smaller than Size/size_t.
I think you were trying to mention that off_t could be wider than
size_t and you're right in that point. I thought that it is safe since
we are trying to read the whole content of the file at once here into
palloc'ed memory.
However, on second thought, if st_size is out of the range of ssize_t,
and palloc accepts that size, at least on Linux, read(2) reads only
0x7ffff000 bytes and raches the error reporting. Addition to that,
this size was closer to the memory allocation size limit than I had
thought.
As the result, I removed the change. However, I kept the change of the
type of variable "r" and corresponding placeholder %zd.
> > diff --git a/src/backend/libpq/be-secure-gssapi.c
> > b/src/backend/libpq/be-secure-gssapi.c
> > index bc04e78abb..68645b4519 100644
> > --- a/src/backend/libpq/be-secure-gssapi.c
> > +++ b/src/backend/libpq/be-secure-gssapi.c
> > @@ -572,9 +572,9 @@ secure_open_gssapi(Port *port)
> > if (input.length > PQ_GSS_RECV_BUFFER_SIZE)
> > {
> > ereport(COMMERROR,
> > - (errmsg("oversize GSSAPI packet sent
> > - by the client (%zu > %d)",
> > + (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)",
> > (size_t) input.length,
> > -
> > PQ_GSS_RECV_BUFFER_SIZE)));
> > + (size_t) PQ_GSS_RECV_BUFFER_SIZE)));
> > return -1;
> > }
> >
>
> Might be better to add that cast to the definition of
> PQ_GSS_RECV_BUFFER_SIZE instead, so that all code can benefit.
As far as I see, the only exceptional uses I found were a comparison
with int values, and being passed as an OM_uint32 (to one of the
parameters of gss_wrap_size_limit()). Therefore, I agree that it is
beneficial. By the way, we currently define Size as the same as size_t
(since 1998). Is it correct to consider Size as merely for backward
compatibility and we should use size_t for new code? I used size_t in
the modified part in the attached patch.
> > diff --git a/src/backend/replication/repl_gram.y
> > b/src/backend/replication/repl_gram.y
> > index 7474f5bd67..baa76280b9 100644
> > --- a/src/backend/replication/repl_gram.y
> > +++ b/src/backend/replication/repl_gram.y
> > @@ -312,11 +312,6 @@ timeline_history:
> > {
> > TimeLineHistoryCmd *cmd;
> > - if ($2 <= 0)
> > - ereport(ERROR,
> > -
> > (errcode(ERRCODE_SYNTAX_ERROR),
> > - errmsg("invalid
> > - timeline %u",
> > - $2)));
> > -
...
> I don't think this is correct. It loses the check for == 0.
Ugh. It's my mistake. So we need to consider unifying the messages
again. In walsummaryfuncs.c, %lld is required, but it's silly for the
uses in repl_gram.y. Finally, I chose not to change anything here.
> > diff --git a/src/backend/tsearch/to_tsany.c
> > b/src/backend/tsearch/to_tsany.c
> > index 88cba58cba..9d21178107 100644
> > --- a/src/backend/tsearch/to_tsany.c
> > +++ b/src/backend/tsearch/to_tsany.c
> > @@ -191,7 +191,8 @@ make_tsvector(ParsedText *prs)
> > if (lenstr > MAXSTRPOS)
> > ereport(ERROR,
> > (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
> > - errmsg("string is too long for tsvector (%d
> > - bytes, max %d bytes)", lenstr, MAXSTRPOS)));
> > + /* cast values to avoid extra translatable messages */
> > + errmsg("string is too long for tsvector (%ld bytes, max %ld bytes)",
> > (long) lenstr, (long) MAXSTRPOS)));
> > totallen = CALCDATASIZE(prs->curwords, lenstr);
> > in = (TSVector) palloc0(totallen);
>
> I think it would be better instead to change the message in
> tsvectorin() to *not* use long. The size of long is unportable, so I
> would rather avoid using it at all.
The casts to long are tentative only to adjust to the corresponding
placeholder, and in this context, portability concerns are not
applicable. However, those casts are apparently useless. As you
suggested, I tried to change tsvectorin() instead, but there's a
problem here.
tsvector.c:224
> errmsg("string is too long for tsvector (%ld bytes, max %ld bytes)",
> (long) (cur - tmpbuf), (long) MAXSTRPOS)));
cur and tmpbuf are pointers. The byte width of the subtraction results
varies by architecture. However, the surrounding code apparently
assumes that the difference fits within an int. I added a cast to int
for the pointer arithmetic here. (Although I'm not sure this is the
right direction.)
> > diff --git a/src/backend/utils/adt/varlena.c
> > b/src/backend/utils/adt/varlena.c
> > index 8d28dd42ce..5de490b569 100644
> > --- a/src/backend/utils/adt/varlena.c
> > +++ b/src/backend/utils/adt/varlena.c
> > @@ -3217,8 +3217,9 @@ byteaGetByte(PG_FUNCTION_ARGS)
> > if (n < 0 || n >= len)
> > ereport(ERROR,
> > (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR),
> > - errmsg("index %d out of valid range, 0..%d",
> > - n, len - 1)));
> > + /* cast values to avoid extra translable messages */
> > + errmsg("index %lld out of valid range, 0..%lld",
> > + (long long)n, (long long) len - 1)));
> > byte = ((unsigned char *) VARDATA_ANY(v))[n];
>
> I think this is taking it too far. We shouldn't try to make all
> similar messages use the same placeholders. If the underlying types
> are different, we should use them. Adding more casts makes the code
> less robust overall. The size_t/ssize_t cleanup is different, because
> there the types were arguably wrong to begin with, and by using the
> right types we move toward more consistency.
Ouch! Understood. They treat byte and bit locations accordingly. I
agree that it's too far. Removed.
> > diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c
> > b/src/bin/pg_combinebackup/pg_combinebackup.c
> > index 6f0814d9ac..feb4d5dcf4 100644
> > --- a/src/bin/pg_combinebackup/pg_combinebackup.c
> > +++ b/src/bin/pg_combinebackup/pg_combinebackup.c
> > - pg_fatal("could not read file \"%s\": read only %zd of %lld
> > bytes",
> > - filename, rb, (long long int) st.st_size);
> > + /* cast st_size to avoid extra translatable messages */
> > + pg_fatal("could not read file \"%s\": read only %zd of %zu
> > bytes",
> > + filename, rb, (size_t) st.st_size);
> > }
> > /* Adjust buffer length for new data and restore trailing-\0
> > invariant */
>
> Similar to above, casting off_t to size_t is dubious.
The same discussion regarding the change in twophase.c is also
applicable to this change. I applied the same amendment.
> > diff --git a/src/port/user.c b/src/port/user.c
> > index 7444aeb64b..9364bdb69e 100644
> > --- a/src/port/user.c
> > +++ b/src/port/user.c
> > @@ -40,8 +40,8 @@ pg_get_user_name(uid_t user_id, char *buffer, size_t
> > buflen)
> > }
> > if (pwerr != 0)
> > snprintf(buffer, buflen,
> > - _("could not look up local user ID %d: %s"),
> > - (int) user_id,
> > + _("could not look up local user ID %ld: %s"),
> > + (long) user_id,
> > strerror_r(pwerr, pwdbuf, sizeof(pwdbuf)));
> > else
> > snprintf(buffer, buflen,
>
> Also dubious use of "long" here.
Okay, used %d instead. In addition to that, I removed the casts from
uid_t expecting that compilers will detect the change of the
definition of uid_t.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 8090ac9fc1..e2cde8c570 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1310,7 +1310,7 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
uint32 crc_offset;
pg_crc32c calc_crc,
file_crc;
- int r;
+ ssize_t r;
TwoPhaseFilePath(path, xid);
@@ -1368,8 +1368,13 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
(errcode_for_file_access(),
errmsg("could not read file \"%s\": %m", path)));
else
+ /*
+ * We may get here with st_size out of the range of ssize_t and
+ * even size_t. Therefore, we need to use %lld for the file size
+ * here.
+ */
ereport(ERROR,
- (errmsg("could not read file \"%s\": read %d of %lld",
+ (errmsg("could not read file \"%s\": read %zd of %lld",
path, r, (long long int) stat.st_size)));
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 20a5f86209..3331f23f69 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -3385,7 +3385,7 @@ XLogFileCopy(TimeLineID destTLI, XLogSegNo destsegno,
*/
for (nbytes = 0; nbytes < wal_segment_size; nbytes += sizeof(buffer))
{
- int nread;
+ ssize_t nread;
nread = upto - nbytes;
@@ -3398,7 +3398,7 @@ XLogFileCopy(TimeLineID destTLI, XLogSegNo destsegno,
if (nread > 0)
{
- int r;
+ ssize_t r;
if (nread > sizeof(buffer))
nread = sizeof(buffer);
@@ -3414,7 +3414,7 @@ XLogFileCopy(TimeLineID destTLI, XLogSegNo destsegno,
else
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
path, r, (Size) nread)));
}
pgstat_report_wait_end();
@@ -4250,7 +4250,7 @@ ReadControlFile(void)
pg_crc32c crc;
int fd;
static char wal_segsz_str[20];
- int r;
+ ssize_t r;
/*
* Read data...
@@ -4275,7 +4275,7 @@ ReadControlFile(void)
else
ereport(PANIC,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
XLOG_CONTROL_FILE, r, sizeof(ControlFileData))));
}
pgstat_report_wait_end();
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 2b607c5270..3141453656 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1883,8 +1883,8 @@ auth_peer(hbaPort *port)
int save_errno = errno;
ereport(LOG,
- (errmsg("could not look up local user ID %ld: %s",
- (long) uid,
+ (errmsg("could not look up local user ID %d: %s",
+ uid,
save_errno ? strerror(save_errno) : _("user does not exist"))));
return STATUS_ERROR;
}
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
index bc04e78abb..3d7ddf7176 100644
--- a/src/backend/libpq/be-secure-gssapi.c
+++ b/src/backend/libpq/be-secure-gssapi.c
@@ -48,8 +48,8 @@
* Therefore, these two #define's are effectively part of the protocol
* spec and can't ever be changed.
*/
-#define PQ_GSS_SEND_BUFFER_SIZE 16384
-#define PQ_GSS_RECV_BUFFER_SIZE 16384
+#define PQ_GSS_SEND_BUFFER_SIZE ((size_t) 16384)
+#define PQ_GSS_RECV_BUFFER_SIZE ((size_t) 16384)
/*
* Since we manage at most one GSS-encrypted connection per backend,
@@ -572,7 +572,7 @@ secure_open_gssapi(Port *port)
if (input.length > PQ_GSS_RECV_BUFFER_SIZE)
{
ereport(COMMERROR,
- (errmsg("oversize GSSAPI packet sent by the client (%zu > %d)",
+ (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)",
(size_t) input.length,
PQ_GSS_RECV_BUFFER_SIZE)));
return -1;
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index a529da983a..f56f704aee 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -700,7 +700,7 @@ StartupReplicationOrigin(void)
{
const char *path = "pg_logical/replorigin_checkpoint";
int fd;
- int readBytes;
+ ssize_t readBytes;
uint32 magic = REPLICATION_STATE_MAGIC;
int last_state = 0;
pg_crc32c file_crc;
@@ -747,7 +747,7 @@ StartupReplicationOrigin(void)
else
ereport(PANIC,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
path, readBytes, sizeof(magic))));
}
COMP_CRC32C(crc, &magic, sizeof(magic));
@@ -786,7 +786,7 @@ StartupReplicationOrigin(void)
{
ereport(PANIC,
(errcode_for_file_access(),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
path, readBytes, sizeof(disk_state))));
}
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index ac24b51860..729b80385a 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -2016,7 +2016,7 @@ snapshot_not_interesting:
static void
SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path)
{
- int readBytes;
+ ssize_t readBytes;
pgstat_report_wait_start(WAIT_EVENT_SNAPBUILD_READ);
readBytes = read(fd, dest, size);
@@ -2037,7 +2037,7 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path)
else
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
path, readBytes, size)));
}
}
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 91ca397857..09422a1f7d 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -2156,7 +2156,7 @@ RestoreSlotFromDisk(const char *name)
char path[MAXPGPATH + 22];
int fd;
bool restored = false;
- int readBytes;
+ ssize_t readBytes;
pg_crc32c checksum;
/* no need to lock here, no concurrent access allowed yet */
@@ -2215,7 +2215,7 @@ RestoreSlotFromDisk(const char *name)
else
ereport(PANIC,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
path, readBytes,
(Size) ReplicationSlotOnDiskConstantSize)));
}
@@ -2256,7 +2256,7 @@ RestoreSlotFromDisk(const char *name)
else
ereport(PANIC,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
path, readBytes, (Size) cp.length)));
}
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index bc40c454de..706903b9fa 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -644,7 +644,7 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
while (bytesleft > 0)
{
PGAlignedBlock rbuf;
- int nread;
+ ssize_t nread;
pgstat_report_wait_start(WAIT_EVENT_WALSENDER_TIMELINE_HISTORY_READ);
nread = read(fd, rbuf.data, sizeof(rbuf));
@@ -657,7 +657,7 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
else if (nread == 0)
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
path, nread, (Size) bytesleft)));
pq_sendbytes(&buf, rbuf.data, nread);
diff --git a/src/backend/utils/adt/tsvector.c b/src/backend/utils/adt/tsvector.c
index 4c6a15757a..7d2a3589b0 100644
--- a/src/backend/utils/adt/tsvector.c
+++ b/src/backend/utils/adt/tsvector.c
@@ -213,15 +213,19 @@ tsvectorin(PG_FUNCTION_ARGS)
if (toklen >= MAXSTRLEN)
ereturn(escontext, (Datum) 0,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("word is too long (%ld bytes, max %ld bytes)",
- (long) toklen,
- (long) (MAXSTRLEN - 1))));
+ errmsg("word is too long (%d bytes, max %d bytes)",
+ toklen, MAXSTRLEN - 1)));
if (cur - tmpbuf > MAXSTRPOS)
ereturn(escontext, (Datum) 0,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("string is too long for tsvector (%ld bytes, max %ld bytes)",
- (long) (cur - tmpbuf), (long) MAXSTRPOS)));
+ /*
+ * Cast the pointer arithmetic to avoid extra translatable
+ * messages. We are assuming that the result fits within
+ * an int here.
+ */
+ errmsg("string is too long for tsvector (%d bytes, max %d bytes)",
+ (int) (cur - tmpbuf), MAXSTRPOS)));
/*
* Enlarge buffers if needed
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index 687adcbd69..12e33fccfb 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -935,8 +935,8 @@ tsvector_concat(PG_FUNCTION_ARGS)
i,
j,
i1,
- i2,
- dataoff,
+ i2;
+ size_t dataoff,
output_bytes,
output_size;
char *data,
@@ -1123,7 +1123,7 @@ tsvector_concat(PG_FUNCTION_ARGS)
if (dataoff > MAXSTRPOS)
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("string is too long for tsvector (%d bytes, max %d bytes)", dataoff, MAXSTRPOS)));
+ errmsg("string is too long for tsvector (%zu bytes, max %zu bytes)", dataoff, MAXSTRPOS)));
/*
* Adjust sizes (asserting that we didn't overrun the original estimates)
diff --git a/src/backend/utils/cache/relmapper.c b/src/backend/utils/cache/relmapper.c
index 48d344ae3f..6d118a1da7 100644
--- a/src/backend/utils/cache/relmapper.c
+++ b/src/backend/utils/cache/relmapper.c
@@ -786,7 +786,7 @@ read_relmap_file(RelMapFile *map, char *dbpath, bool lock_held, int elevel)
char mapfilename[MAXPGPATH];
pg_crc32c crc;
int fd;
- int r;
+ ssize_t r;
Assert(elevel >= ERROR);
@@ -830,7 +830,7 @@ read_relmap_file(RelMapFile *map, char *dbpath, bool lock_held, int elevel)
else
ereport(elevel,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
mapfilename, r, sizeof(RelMapFile))));
}
pgstat_report_wait_end();
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index 555f0175f0..c1430e7950 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -329,9 +329,9 @@ FindStreamingStart(uint32 *tli)
{
int fd;
char buf[4];
- int bytes_out;
+ size_t bytes_out;
char fullpath[MAXPGPATH * 2];
- int r;
+ ssize_t r;
snprintf(fullpath, sizeof(fullpath), "%s/%s", basedir, dirent->d_name);
@@ -349,7 +349,7 @@ FindStreamingStart(uint32 *tli)
pg_fatal("could not read compressed file \"%s\": %m",
fullpath);
else
- pg_fatal("could not read compressed file \"%s\": read %d of %zu",
+ pg_fatal("could not read compressed file \"%s\": read %zd of %zu",
fullpath, r, sizeof(buf));
}
@@ -359,7 +359,7 @@ FindStreamingStart(uint32 *tli)
if (bytes_out != WalSegSz)
{
- pg_log_warning("compressed segment file \"%s\" has incorrect uncompressed size %d, skipping",
+ pg_log_warning("compressed segment file \"%s\" has incorrect uncompressed size %zu, skipping",
dirent->d_name, bytes_out);
continue;
}
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 74f8be9eea..69c5860604 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -1298,6 +1298,11 @@ slurp_file(int fd, char *filename, StringInfo buf, int maxlen)
if (rb < 0)
pg_fatal("could not read file \"%s\": %m", filename);
else
+ /*
+ * We may get here with st_size out of the range of ssize_t and
+ * even size_t. Therefore, we need to use %lld for the file size
+ * here.
+ */
pg_fatal("could not read file \"%s\": read only %zd of %lld bytes",
filename, rb, (long long int) st.st_size);
}
diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c
index 41f06bb26b..c1617bb826 100644
--- a/src/bin/pg_combinebackup/reconstruct.c
+++ b/src/bin/pg_combinebackup/reconstruct.c
@@ -504,15 +504,16 @@ make_rfile(char *filename, bool missing_ok)
static void
read_bytes(rfile *rf, void *buffer, unsigned length)
{
- int rb = read(rf->fd, buffer, length);
+ ssize_t rb = read(rf->fd, buffer, length);
if (rb != length)
{
if (rb < 0)
pg_fatal("could not read file \"%s\": %m", rf->filename);
else
- pg_fatal("could not read file \"%s\": read only %d of %u bytes",
- rf->filename, rb, length);
+ /* cast length to avoid extra translatable messages */
+ pg_fatal("could not read file \"%s\": read only %zd of %lld bytes",
+ rf->filename, rb, (long long int) length);
}
}
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index 73932504ca..82713534c0 100644
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -62,8 +62,9 @@ cloneFile(const char *src, const char *dst,
unlink(dst);
- pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %s",
- schemaName, relName, src, dst, strerror(save_errno));
+ errno = save_errno;
+ pg_fatal("error while cloning relation \"%s.%s\" (\"%s\" to \"%s\"): %m",
+ schemaName, relName, src, dst);
}
close(src_fd);
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9b0fa041f7..aa0b63121e 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -610,8 +610,8 @@ exec_command_cd(PsqlScanState scan_state, bool active_branch, const char *cmd)
dir = pw->pw_dir;
else
{
- pg_log_error("could not get home directory for user ID %ld: %s",
- (long) user_id,
+ pg_log_error("could not get home directory for user ID %d: %s",
+ user_id,
errno ? strerror(errno) : _("user does not exist"));
success = false;
}
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 82309b2510..5136f33205 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -70,7 +70,7 @@ get_controlfile_by_exact_path(const char *ControlFilePath, bool *crc_ok_p)
ControlFileData *ControlFile;
int fd;
pg_crc32c crc;
- int r;
+ ssize_t r;
#ifdef FRONTEND
pg_crc32c last_crc;
int retries = 0;
@@ -113,10 +113,14 @@ retry:
#ifndef FRONTEND
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
- errmsg("could not read file \"%s\": read %d of %zu",
+ /*
+ * cast the sizeof reulst to avoid extra translatable
+ * messages
+ */
+ errmsg("could not read file \"%s\": read %zd of %zu",
ControlFilePath, r, sizeof(ControlFileData))));
#else
- pg_fatal("could not read file \"%s\": read %d of %zu",
+ pg_fatal("could not read file \"%s\": read %zd of %zu",
ControlFilePath, r, sizeof(ControlFileData));
#endif
}
diff --git a/src/common/username.c b/src/common/username.c
index acaeb13d2e..a488b10317 100644
--- a/src/common/username.c
+++ b/src/common/username.c
@@ -40,8 +40,8 @@ get_user_name(char **errstr)
pw = getpwuid(user_id);
if (!pw)
{
- *errstr = psprintf(_("could not look up effective user ID %ld: %s"),
- (long) user_id,
+ *errstr = psprintf(_("could not look up effective user ID %d: %s"),
+ user_id,
errno ? strerror(errno) : _("user does not exist"));
return NULL;
}
diff --git a/src/port/user.c b/src/port/user.c
index 7444aeb64b..985e3605a0 100644
--- a/src/port/user.c
+++ b/src/port/user.c
@@ -41,12 +41,12 @@ pg_get_user_name(uid_t user_id, char *buffer, size_t buflen)
if (pwerr != 0)
snprintf(buffer, buflen,
_("could not look up local user ID %d: %s"),
- (int) user_id,
+ user_id,
strerror_r(pwerr, pwdbuf, sizeof(pwdbuf)));
else
snprintf(buffer, buflen,
_("local user with ID %d does not exist"),
- (int) user_id);
+ user_id);
return false;
}
@@ -77,12 +77,12 @@ pg_get_user_home_dir(uid_t user_id, char *buffer, size_t buflen)
if (pwerr != 0)
snprintf(buffer, buflen,
_("could not look up local user ID %d: %s"),
- (int) user_id,
+ user_id,
strerror_r(pwerr, pwdbuf, sizeof(pwdbuf)));
else
snprintf(buffer, buflen,
_("local user with ID %d does not exist"),
- (int) user_id);
+ user_id);
return false;
}
