Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 98c6231

Browse files
committed
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
1 parent da083b2 commit 98c6231

File tree

3 files changed

+15
-15
lines changed

3 files changed

+15
-15
lines changed

src/backend/access/transam/xlog.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -2280,7 +2280,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible)
22802280
char *from;
22812281
Size nbytes;
22822282
Size nleft;
2283-
int written;
2283+
ssize_t written;
22842284
instr_time start;
22852285

22862286
/* OK to write the page(s) */

src/backend/backup/basebackup.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ static void perform_base_backup(basebackup_options *opt, bbsink *sink,
117117
IncrementalBackupInfo *ib);
118118
static void parse_basebackup_options(List *options, basebackup_options *opt);
119119
static int compareWalFileNames(const ListCell *a, const ListCell *b);
120-
static int basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
121-
const char *filename, bool partial_read_ok);
120+
static ssize_t basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
121+
const char *filename, bool partial_read_ok);
122122

123123
/* Was the backup currently in-progress initiated in recovery mode? */
124124
static bool backup_started_in_recovery = false;
@@ -525,7 +525,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink,
525525
{
526526
char *walFileName = (char *) lfirst(lc);
527527
int fd;
528-
size_t cnt;
528+
ssize_t cnt;
529529
pgoff_t len = 0;
530530

531531
snprintf(pathbuf, MAXPGPATH, XLOGDIR "/%s", walFileName);
@@ -2079,11 +2079,11 @@ convert_link_to_directory(const char *pathbuf, struct stat *statbuf)
20792079
*
20802080
* Returns the number of bytes read.
20812081
*/
2082-
static int
2082+
static ssize_t
20832083
basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
20842084
const char *filename, bool partial_read_ok)
20852085
{
2086-
int rc;
2086+
ssize_t rc;
20872087

20882088
pgstat_report_wait_start(WAIT_EVENT_BASEBACKUP_READ);
20892089
rc = pg_pread(fd, buf, nbytes, offset);
@@ -2096,7 +2096,7 @@ basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset,
20962096
if (!partial_read_ok && rc > 0 && rc != nbytes)
20972097
ereport(ERROR,
20982098
(errcode_for_file_access(),
2099-
errmsg("could not read file \"%s\": read %d of %zu",
2099+
errmsg("could not read file \"%s\": read %zd of %zu",
21002100
filename, rc, nbytes)));
21012101

21022102
return rc;

src/bin/pg_combinebackup/reconstruct.c

+8-8
Original file line numberDiff line numberDiff line change
@@ -504,15 +504,15 @@ make_rfile(char *filename, bool missing_ok)
504504
static void
505505
read_bytes(rfile *rf, void *buffer, unsigned length)
506506
{
507-
unsigned rb = read(rf->fd, buffer, length);
507+
int rb = read(rf->fd, buffer, length);
508508

509509
if (rb != length)
510510
{
511511
if (rb < 0)
512512
pg_fatal("could not read file \"%s\": %m", rf->filename);
513513
else
514514
pg_fatal("could not read file \"%s\": read only %d of %d bytes",
515-
rf->filename, (int) rb, length);
515+
rf->filename, rb, length);
516516
}
517517
}
518518

@@ -614,7 +614,7 @@ write_reconstructed_file(char *input_filename,
614614
{
615615
uint8 buffer[BLCKSZ];
616616
rfile *s = sourcemap[i];
617-
unsigned wb;
617+
int wb;
618618

619619
/* Update accounting information. */
620620
if (s == NULL)
@@ -641,7 +641,7 @@ write_reconstructed_file(char *input_filename,
641641
}
642642
else
643643
{
644-
unsigned rb;
644+
int rb;
645645

646646
/* Read the block from the correct source, except if dry-run. */
647647
rb = pg_pread(s->fd, buffer, BLCKSZ, offsetmap[i]);
@@ -650,9 +650,9 @@ write_reconstructed_file(char *input_filename,
650650
if (rb < 0)
651651
pg_fatal("could not read file \"%s\": %m", s->filename);
652652
else
653-
pg_fatal("could not read file \"%s\": read only %d of %d bytes at offset %u",
654-
s->filename, (int) rb, BLCKSZ,
655-
(unsigned) offsetmap[i]);
653+
pg_fatal("could not read file \"%s\": read only %d of %d bytes at offset %llu",
654+
s->filename, rb, BLCKSZ,
655+
(unsigned long long) offsetmap[i]);
656656
}
657657
}
658658

@@ -663,7 +663,7 @@ write_reconstructed_file(char *input_filename,
663663
pg_fatal("could not write file \"%s\": %m", output_filename);
664664
else
665665
pg_fatal("could not write file \"%s\": wrote only %d of %d bytes",
666-
output_filename, (int) wb, BLCKSZ);
666+
output_filename, wb, BLCKSZ);
667667
}
668668

669669
/* Update the checksum computation. */

0 commit comments

Comments
 (0)