aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2023-12-27 11:02:53 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2023-12-27 11:02:53 -0500
commit98c6231d198a98b1c5ec9d400cd5593752fa9de9 (patch)
tree22db8f47db3b763b3210afd5ce135d39b9b065d1 /src
parentda083b20f63739130dd65d8f0ca4a49b6db31cfe (diff)
downloadpostgresql-98c6231d198a98b1c5ec9d400cd5593752fa9de9.tar.gz
postgresql-98c6231d198a98b1c5ec9d400cd5593752fa9de9.zip
Fix incorrect data type choices in some read and write calls.
Recently-introduced code in reconstruct.c was using "unsigned" to store the result of read(), pg_pread(), or write(). This is completely bogus: it breaks subsequent tests for the result being negative, as we're being reminded of by a chorus of buildfarm warnings. Switch to "int" as was doubtless intended. (There are several other uses of "unsigned" in this file that also look poorly chosen to me, but for now I'm just trying to clean up the buildfarm.) A larger problem is that "int" is not necessarily wide enough to hold the result: per POSIX, all these functions return ssize_t. In places where the requested read or write length clearly fits in int, that's academic. It may be academic anyway as long as we constrain individual data files to 1GB, since even a readv or writev-like operation would then not be responsible for transferring more than 1GB. Nonetheless it seems like trouble waiting to happen, so I made a pass over readv and writev calls and fixed the result variables where that seemed appropriate. We might want to think about changing some of the fd.c functions to return ssize_t too, for future-proofing; but I didn't tackle that here. Discussion: https://postgr.es/m/1672202.1703441340@sss.pgh.pa.us
Diffstat (limited to 'src')
-rw-r--r--src/backend/access/transam/xlog.c2
-rw-r--r--src/backend/backup/basebackup.c12
-rw-r--r--src/bin/pg_combinebackup/reconstruct.c16
3 files changed, 15 insertions, 15 deletions
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 1e9019156a5..1264849883b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -2280,7 +2280,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
char *from;
Size nbytes;
Size nleft;
- int written;
+ ssize_t written;
instr_time start;
/* OK to write the page(s) */
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 5ee9628422e..84246739ae6 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -117,8 +117,8 @@ static void perform_base_backup(basebackup_options *opt, bbsink *sink,
IncrementalBackupInfo *ib);
static void parse_basebackup_options(List *options, basebackup_options *opt);
static int compareWalFileNames(const ListCell *a, const ListCell *b);
-static int basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
- const char *filename, bool partial_read_ok);
+static ssize_t basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
+ const char *filename, bool partial_read_ok);
/* Was the backup currently in-progress initiated in recovery mode? */
static bool backup_started_in_recovery = false;
@@ -525,7 +525,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
{
char *walFileName = (char *) lfirst(lc);
int fd;
- size_t cnt;
+ ssize_t cnt;
pgoff_t len = 0;
snprintf(pathbuf, MAXPGPATH, XLOGDIR "/%s", walFileName);
@@ -2079,11 +2079,11 @@ convert_link_to_directory(const char *pathbuf, struct stat *statbuf)
*
* Returns the number of bytes read.
*/
-static int
+static ssize_t
basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
const char *filename, bool partial_read_ok)
{
- int rc;
+ ssize_t rc;
pgstat_report_wait_start(WAIT_EVENT_BASEBACKUP_READ);
rc = pg_pread(fd, buf, nbytes, offset);
@@ -2096,7 +2096,7 @@ basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
if (!partial_read_ok && rc > 0 && rc != nbytes)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not read file \"%s\": read %d of %zu",
+ errmsg("could not read file \"%s\": read %zd of %zu",
filename, rc, nbytes)));
return rc;
diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c
index 6decdd89340..874e6cd150b 100644
--- a/src/bin/pg_combinebackup/reconstruct.c
+++ b/src/bin/pg_combinebackup/reconstruct.c
@@ -504,7 +504,7 @@ make_rfile(char *filename, bool missing_ok)
static void
read_bytes(rfile *rf, void *buffer, unsigned length)
{
- unsigned rb = read(rf->fd, buffer, length);
+ int rb = read(rf->fd, buffer, length);
if (rb != length)
{
@@ -512,7 +512,7 @@ read_bytes(rfile *rf, void *buffer, unsigned length)
pg_fatal("could not read file \"%s\": %m", rf->filename);
else
pg_fatal("could not read file \"%s\": read only %d of %d bytes",
- rf->filename, (int) rb, length);
+ rf->filename, rb, length);
}
}
@@ -614,7 +614,7 @@ write_reconstructed_file(char *input_filename,
{
uint8 buffer[BLCKSZ];
rfile *s = sourcemap[i];
- unsigned wb;
+ int wb;
/* Update accounting information. */
if (s == NULL)
@@ -641,7 +641,7 @@ write_reconstructed_file(char *input_filename,
}
else
{
- unsigned rb;
+ int rb;
/* Read the block from the correct source, except if dry-run. */
rb = pg_pread(s->fd, buffer, BLCKSZ, offsetmap[i]);
@@ -650,9 +650,9 @@ write_reconstructed_file(char *input_filename,
if (rb < 0)
pg_fatal("could not read file \"%s\": %m", s->filename);
else
- pg_fatal("could not read file \"%s\": read only %d of %d bytes at offset %u",
- s->filename, (int) rb, BLCKSZ,
- (unsigned) offsetmap[i]);
+ pg_fatal("could not read file \"%s\": read only %d of %d bytes at offset %llu",
+ s->filename, rb, BLCKSZ,
+ (unsigned long long) offsetmap[i]);
}
}
@@ -663,7 +663,7 @@ write_reconstructed_file(char *input_filename,
pg_fatal("could not write file \"%s\": %m", output_filename);
else
pg_fatal("could not write file \"%s\": wrote only %d of %d bytes",
- output_filename, (int) wb, BLCKSZ);
+ output_filename, wb, BLCKSZ);
}
/* Update the checksum computation. */