From: Paul Eggert Date: Fri, 1 Nov 2024 16:40:36 +0000 (-0700) Subject: Adjust better to Gnulib signed-int read changes X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7c0feaefd0355fb99a46858d2d66d42966a0f6c5;p=thirdparty%2Ftar.git Adjust better to Gnulib signed-int read changes The 2024-08-09 Gnulib changes that caused some modules prefer signed types to size_t means that Tar should follow suit. * src/buffer.c (short_read): * src/system.c (sys_child_open_for_compress) (sys_child_open_for_uncompress): rmtread and safe_read return ptrdiff_t not idx_t; don’t rely on implementation defined conversion. * src/misc.c (blocking_read): Never return a negative number. Return idx_t, not ptrdiff_t, with the same convention for EOF and error as the new full_read. All callers changed. * src/sparse.c (sparse_dump_region, check_sparse_region) (check_data_region): * src/update.c (append_file): full_read no longer returns SAFE_READ_ERROR for I/O error; instead it returns the number of bytes successfully read, and sets errno. Adjust to this. * src/system.c (sys_child_open_for_uncompress): Rewrite to avoid need for goto and label. --- diff --git a/src/buffer.c b/src/buffer.c index 242f92eb..4f5e4faa 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -987,8 +987,12 @@ short_read (idx_t status) || (left && status && read_full_records)) { if (status) - while ((status = rmtread (archive, more, left)) == SAFE_READ_ERROR) - archive_read_error (); + { + ptrdiff_t nread; + while ((nread = rmtread (archive, more, left)) < 0) + archive_read_error (); + status = nread; + } if (status == 0) break; diff --git a/src/common.h b/src/common.h index 95004597..5680f4df 100644 --- a/src/common.h +++ b/src/common.h @@ -731,7 +731,7 @@ void undo_last_backup (void); int deref_stat (char const *name, struct stat *buf); -ptrdiff_t blocking_read (int fd, void *buf, idx_t count); +idx_t blocking_read (int fd, void *buf, idx_t count); idx_t blocking_write (int fd, void const *buf, idx_t count); extern idx_t chdir_current; diff --git a/src/compare.c b/src/compare.c index de664ab4..71f3df61 100644 --- a/src/compare.c +++ b/src/compare.c @@ -85,11 +85,11 @@ process_noop (MAYBE_UNUSED idx_t size, MAYBE_UNUSED char *data) static bool process_rawdata (idx_t bytes, char *buffer) { - ptrdiff_t status = blocking_read (diff_handle, diff_buffer, bytes); + idx_t status = blocking_read (diff_handle, diff_buffer, bytes); - if (status != bytes) + if (status < bytes) { - if (status < 0) + if (errno) { read_error (current_stat_info.file_name); report_difference (¤t_stat_info, NULL); diff --git a/src/create.c b/src/create.c index 0af30214..fac7c701 100644 --- a/src/create.c +++ b/src/create.c @@ -1050,25 +1050,16 @@ dump_regular_file (int fd, struct tar_stat_info *st) memset (blk->buffer + size_left, 0, BLOCKSIZE - beyond); } - ptrdiff_t count; - if (fd <= 0) - count = bufsize; - else - { - count = blocking_read (fd, blk->buffer, bufsize); - if (count < 0) - { - read_diag_details (st->orig_file_name, - st->stat.st_size - size_left, bufsize); - pad_archive (size_left); - return dump_status_short; - } - } + idx_t count = (fd <= 0 ? bufsize + : blocking_read (fd, blk->buffer, bufsize)); size_left -= count; set_next_block_after (blk + (bufsize - 1) / BLOCKSIZE); if (count != bufsize) { + if (errno) + read_diag_details (st->orig_file_name, + st->stat.st_size - size_left, bufsize); memset (blk->buffer + count, 0, bufsize - count); warnopt (WARN_FILE_SHRANK, 0, ngettext (("%s: File shrank by %jd byte;" diff --git a/src/misc.c b/src/misc.c index b1bafcf5..c6f01f79 100644 --- a/src/misc.c +++ b/src/misc.c @@ -841,25 +841,29 @@ deref_stat (char const *name, struct stat *buf) /* Read from FD into the buffer BUF with COUNT bytes. Attempt to fill BUF. Wait until input is available; this matters because files are opened O_NONBLOCK for security reasons, and on some file systems - this can cause read to fail with errno == EAGAIN. Return the - actual number of bytes read, zero for EOF, or - -1 upon error. */ -ptrdiff_t + this can cause read to fail with errno == EAGAIN. + If returning less than COUNT, set errno to indicate the error + except set errno = 0 to indicate EOF. */ +idx_t blocking_read (int fd, void *buf, idx_t count) { idx_t bytes = full_read (fd, buf, count); #if defined F_SETFL && O_NONBLOCK - if (bytes == SAFE_READ_ERROR && errno == EAGAIN) + if (bytes < count && errno == EAGAIN) { int flags = fcntl (fd, F_GETFL); if (0 <= flags && flags & O_NONBLOCK && fcntl (fd, F_SETFL, flags & ~O_NONBLOCK) != -1) - bytes = full_read (fd, buf, count); + { + char *cbuf = buf; + count -= bytes; + bytes += full_read (fd, cbuf + bytes, count); + } } #endif - return bytes == SAFE_READ_ERROR || (bytes == 0 && errno != 0) ? -1 : bytes; + return bytes; } /* Write to FD from the buffer BUF with COUNT bytes. Do a full write. diff --git a/src/sparse.c b/src/sparse.c index de48c6b1..b74984e0 100644 --- a/src/sparse.c +++ b/src/sparse.c @@ -226,12 +226,13 @@ sparse_scan_file_raw (struct tar_sparse_file *file) while (true) { - ptrdiff_t count = blocking_read (fd, buffer, sizeof buffer); - if (count <= 0) + idx_t count = blocking_read (fd, buffer, sizeof buffer); + if (count < sizeof buffer) { - if (count < 0) + if (errno) read_diag_details (st->orig_file_name, offset, sizeof buffer); - break; + if (count == 0) + break; } /* Analyze the block. */ @@ -256,6 +257,8 @@ sparse_scan_file_raw (struct tar_sparse_file *file) } offset += count; + if (count < sizeof buffer) + break; } /* save one more sparse segment of length 0 to indicate that @@ -408,7 +411,6 @@ sparse_select_optab (struct tar_sparse_file *file) static bool sparse_dump_region (struct tar_sparse_file *file, size_t i) { - union block *blk; off_t bytes_left = file->stat_info->sparse_map[i].numbytes; if (!lseek_or_error (file, file->stat_info->sparse_map[i].offset)) @@ -416,58 +418,41 @@ sparse_dump_region (struct tar_sparse_file *file, size_t i) while (bytes_left > 0) { - size_t bufsize = (bytes_left > BLOCKSIZE) ? BLOCKSIZE : bytes_left; - size_t bytes_read; + union block *blk = find_next_block (); + idx_t bufsize = min (bytes_left, BLOCKSIZE); + idx_t bytes_read = full_read (file->fd, blk->buffer, bufsize); + if (bytes_read < BLOCKSIZE) + memset (blk->buffer + bytes_read, 0, BLOCKSIZE - bytes_read); + bytes_left -= bytes_read; + file->dumped_size += bytes_read; - blk = find_next_block (); - bytes_read = full_read (file->fd, blk->buffer, bufsize); - if (bytes_read == SAFE_READ_ERROR) + if (bytes_read < bufsize) { - read_diag_details (file->stat_info->orig_file_name, - (file->stat_info->sparse_map[i].offset - + file->stat_info->sparse_map[i].numbytes - - bytes_left), - bufsize); - return false; - } - else if (bytes_read == 0) - { - if (errno != 0) - { - read_diag_details (file->stat_info->orig_file_name, - (file->stat_info->sparse_map[i].offset + off_t current_offset = (file->stat_info->sparse_map[i].offset + file->stat_info->sparse_map[i].numbytes - - bytes_left), - bufsize); - return false; - } + - bytes_left); + if (errno != 0) + read_diag_details (file->stat_info->orig_file_name, + current_offset, bufsize - bytes_read); else { + off_t cursize = current_offset; struct stat st; - off_t n; - if (fstat (file->fd, &st) == 0) - n = file->stat_info->stat.st_size - st.st_size; - else - n = file->stat_info->stat.st_size - - (file->stat_info->sparse_map[i].offset - + file->stat_info->sparse_map[i].numbytes - - bytes_left); - + if (fstat (file->fd, &st) == 0 && st.st_size < cursize) + cursize = st.st_size; + intmax_t n = file->stat_info->stat.st_size - cursize; warnopt (WARN_FILE_SHRANK, 0, ngettext ("%s: File shrank by %jd byte; padding with zeros", "%s: File shrank by %jd bytes; padding with zeros", n), quotearg_colon (file->stat_info->orig_file_name), - intmax (n)); - if (! ignore_failed_read_option) - set_exit_status (TAREXIT_DIFFERS); - return false; + n); } + if (! ignore_failed_read_option) + set_exit_status (TAREXIT_DIFFERS); + return false; } - memset (blk->buffer + bytes_read, 0, BLOCKSIZE - bytes_read); - bytes_left -= bytes_read; - file->dumped_size += bytes_read; set_next_block_after (blk); } @@ -625,24 +610,14 @@ check_sparse_region (struct tar_sparse_file *file, off_t beg, off_t end) while (beg < end) { - size_t bytes_read; - size_t rdsize = BLOCKSIZE < end - beg ? BLOCKSIZE : end - beg; + idx_t rdsize = min (end - beg, BLOCKSIZE); char diff_buffer[BLOCKSIZE]; - bytes_read = full_read (file->fd, diff_buffer, rdsize); - if (bytes_read == SAFE_READ_ERROR) - { - read_diag_details (file->stat_info->orig_file_name, - beg, - rdsize); - return false; - } - else if (bytes_read == 0) + idx_t bytes_read = full_read (file->fd, diff_buffer, rdsize); + if (bytes_read < rdsize) { - if (errno != 0) - read_diag_details (file->stat_info->orig_file_name, - beg, - rdsize); + if (errno) + read_diag_details (file->stat_info->orig_file_name, beg, rdsize); else report_difference (file->stat_info, _("Size differs")); return false; @@ -675,8 +650,6 @@ check_data_region (struct tar_sparse_file *file, size_t i) while (size_left > 0) { - size_t bytes_read; - size_t rdsize = (size_left > BLOCKSIZE) ? BLOCKSIZE : size_left; char diff_buffer[BLOCKSIZE]; union block *blk = find_next_block (); @@ -687,35 +660,28 @@ check_data_region (struct tar_sparse_file *file, size_t i) } set_next_block_after (blk); file->dumped_size += BLOCKSIZE; - bytes_read = full_read (file->fd, diff_buffer, rdsize); - if (bytes_read == SAFE_READ_ERROR) + idx_t rdsize = min (size_left, BLOCKSIZE); + idx_t bytes_read = full_read (file->fd, diff_buffer, rdsize); + size_left -= bytes_read; + mv_size_left (file->stat_info->archive_file_size - file->dumped_size); + if (memcmp (blk->buffer, diff_buffer, bytes_read) != 0) { - read_diag_details (file->stat_info->orig_file_name, - (file->stat_info->sparse_map[i].offset - + file->stat_info->sparse_map[i].numbytes - - size_left), - rdsize); + report_difference (file->stat_info, _("Contents differ")); return false; } - else if (bytes_read == 0) + + if (bytes_read < rdsize) { if (errno != 0) read_diag_details (file->stat_info->orig_file_name, (file->stat_info->sparse_map[i].offset + file->stat_info->sparse_map[i].numbytes - size_left), - rdsize); + rdsize - bytes_read); else report_difference (¤t_stat_info, _("Size differs")); return false; } - size_left -= bytes_read; - mv_size_left (file->stat_info->archive_file_size - file->dumped_size); - if (memcmp (blk->buffer, diff_buffer, bytes_read)) - { - report_difference (file->stat_info, _("Contents differ")); - return false; - } } return true; } diff --git a/src/system.c b/src/system.c index 944f123f..e2fe1e26 100644 --- a/src/system.c +++ b/src/system.c @@ -452,9 +452,9 @@ sys_child_open_for_compress (void) while (1) { - size_t status = 0; + ptrdiff_t status = 0; char *cursor; - size_t length; + idx_t length; /* Assemble a record. */ @@ -465,7 +465,7 @@ sys_child_open_for_compress (void) size_t size = record_size - length; status = safe_read (STDIN_FILENO, cursor, size); - if (status == SAFE_READ_ERROR) + if (status < 0) read_fatal (use_compress_program_option); if (status == 0) break; @@ -617,34 +617,26 @@ sys_child_open_for_uncompress (void) /* Let's read the archive and pipe it into stdout. */ - while (1) + while (true) { - char *cursor; - size_t maximum; - size_t count; - size_t status; - clear_read_error_count (); - error_loop: - status = rmtread (archive, record_start->buffer, record_size); - if (status == SAFE_READ_ERROR) - { - archive_read_error (); - goto error_loop; - } - if (status == 0) + ptrdiff_t n; + while ((n = rmtread (archive, record_start->buffer, record_size)) < 0) + archive_read_error (); + if (n == 0) break; - cursor = record_start->buffer; - maximum = status; - while (maximum) + + char *cursor = record_start->buffer; + do { - count = maximum < BLOCKSIZE ? maximum : BLOCKSIZE; + idx_t count = min (n, BLOCKSIZE); if (full_write (STDOUT_FILENO, cursor, count) != count) write_error (use_compress_program_option); cursor += count; - maximum -= count; + n -= count; } + while (n); } xclose (STDOUT_FILENO); diff --git a/src/update.c b/src/update.c index 3ce95901..0f602efe 100644 --- a/src/update.c +++ b/src/update.c @@ -56,16 +56,12 @@ append_file (char *file_name) while (true) { union block *start = find_next_block (); - size_t status = full_read (handle, start->buffer, - available_space_after (start)); - if (status == 0) - { - if (errno == 0) - break; - read_fatal (file_name); - } - if (status == SAFE_READ_ERROR) + idx_t bufsize = available_space_after (start); + idx_t status = full_read (handle, start->buffer, bufsize); + if (status < bufsize && errno) read_fatal (file_name); + if (status == 0) + break; if (status % BLOCKSIZE) memset (start->buffer + status - status % BLOCKSIZE, 0, BLOCKSIZE - status % BLOCKSIZE);