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 242f92eb17548a2a26472540f704dd7647eca757..4f5e4faac44f0c94da03fa0c3363ef2e9c6f5220 100644 (file)
--- 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 95004597bef41f1b27a0bc54bcf584be4694035a..5680f4df26cf3392c02c678bedb66359452c3750 100644 (file)
--- 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 de664ab45907b0fbcbf9f65198d12b8bbf8af6f4..71f3df619dae0089c32f60663b7173baa0583383 100644 (file)
--- 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 (&current_stat_info, NULL);

diff --git a/src/create.c b/src/create.c
index 0af30214be8f832263ed74ab0961a99d5a13e24f..fac7c7014a581f5288ce1bedc7ffa12ba72d3bff 100644 (file)
--- 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 b1bafcf59fc29d3913b0816f9266b5e8d8b6d7ea..c6f01f7981cb425a2173aad6c7af494764f3e6a8 100644 (file)
--- 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 de48c6b1784948015122a721018c2fcd51c6fa94..b74984e0ab4fbb11b5c5d36f28c617d39cde0402 100644 (file)
--- 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 (&current_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 944f123f56851ae7b43e94e2782df0eab986d6bd..e2fe1e268711237b454eb683b1cc64db8825099d 100644 (file)
--- 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 3ce959014c0a99f469d21a24a127e0da9b7f2c39..0f602efeb82ac16afdc791cc6b7c623ef86b75cc 100644 (file)
--- 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);