diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2023-12-27 11:02:53 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2023-12-27 11:02:53 -0500 |
commit | 98c6231d198a98b1c5ec9d400cd5593752fa9de9 (patch) | |
tree | 22db8f47db3b763b3210afd5ce135d39b9b065d1 /src/bin/pg_combinebackup/reconstruct.c | |
parent | da083b20f63739130dd65d8f0ca4a49b6db31cfe (diff) | |
download | postgresql-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/bin/pg_combinebackup/reconstruct.c')
-rw-r--r-- | src/bin/pg_combinebackup/reconstruct.c | 16 |
1 files changed, 8 insertions, 8 deletions
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. */ |