tar et al: die if bb_copyfd_size copies less than asked for.

(we have bb_copyfd_exact_size now for that kind of usage)


git-svn-id: svn://busybox.net/trunk/busybox@17038 69ca8d6d-28ef-0310-b511-8ec308f3f277

13 files changed, 85 insertions, 66 deletions

diff --git a/archival/libunarchive/data_extract_all.c b/archival/libunarchive/data_extract_all.c
index 67f8f3534..25bf028d2 100644
--- a/archival/libunarchive/data_extract_all.c
+++ b/archival/libunarchive/data_extract_all.c
@@ -67,10 +67,10 @@ void data_extract_all(archive_handle_t *archive_handle)
                         /* Regular file */
                         dst_fd = xopen3(file_header->name, O_WRONLY | O_CREAT | O_EXCL,
                                                         file_header->mode);
-                        bb_copyfd_size(archive_handle->src_fd, dst_fd, file_header->size);
+                        bb_copyfd_exact_size(archive_handle->src_fd, dst_fd, file_header->size);
                         close(dst_fd);
                         break;
-                        }
+                }
                 case S_IFDIR:
                         res = mkdir(file_header->name, file_header->mode);
                         if ((errno != EISDIR) && (res == -1)
diff --git a/archival/libunarchive/data_extract_to_buffer.c b/archival/libunarchive/data_extract_to_buffer.c
index 95cb8f576..d8fcdf3d3 100644
--- a/archival/libunarchive/data_extract_to_buffer.c
+++ b/archival/libunarchive/data_extract_to_buffer.c
@@ -10,9 +10,8 @@
 
 void data_extract_to_buffer(archive_handle_t *archive_handle)
 {
-        const unsigned int size = archive_handle->file_header->size;
+        unsigned int size = archive_handle->file_header->size;
 
         archive_handle->buffer = xzalloc(size + 1);
-
         xread(archive_handle->src_fd, archive_handle->buffer, size);
 }
diff --git a/archival/libunarchive/data_extract_to_stdout.c b/archival/libunarchive/data_extract_to_stdout.c
index 788246ce7..2e266c046 100644
--- a/archival/libunarchive/data_extract_to_stdout.c
+++ b/archival/libunarchive/data_extract_to_stdout.c
@@ -4,9 +4,11 @@
  */
 
 #include "unarchive.h"
-#include <unistd.h>
+//#include <unistd.h>
 
 void data_extract_to_stdout(archive_handle_t *archive_handle)
 {
-        bb_copyfd_size(archive_handle->src_fd, STDOUT_FILENO, archive_handle->file_header->size);
+        bb_copyfd_exact_size(archive_handle->src_fd,
+                        STDOUT_FILENO,
+                        archive_handle->file_header->size);
 }
diff --git a/archival/libunarchive/get_header_ar.c b/archival/libunarchive/get_header_ar.c
index 7f8c81ca0..6638c65aa 100644
--- a/archival/libunarchive/get_header_ar.c
+++ b/archival/libunarchive/get_header_ar.c
@@ -96,7 +96,8 @@ char get_header_ar(archive_handle_t *archive_handle)
         if (archive_handle->filter(archive_handle) == EXIT_SUCCESS) {
                 archive_handle->action_header(typed);
                 if (archive_handle->sub_archive) {
-                        while (archive_handle->action_data_subarchive(archive_handle->sub_archive) == EXIT_SUCCESS);
+                        while (archive_handle->action_data_subarchive(archive_handle->sub_archive) == EXIT_SUCCESS)
+                                /* repeat */;
                 } else {
                         archive_handle->action_data(archive_handle);
                 }
diff --git a/archival/libunarchive/get_header_tar.c b/archival/libunarchive/get_header_tar.c
index 66c3314a1..beb8687c7 100644
--- a/archival/libunarchive/get_header_tar.c
+++ b/archival/libunarchive/get_header_tar.c
@@ -251,7 +251,7 @@ char get_header_tar(archive_handle_t *archive_handle)
         }
 
         /* Strip trailing '/' in directories */
-        /* Must be done after mode is set as '/' is used to check if its a directory */
+        /* Must be done after mode is set as '/' is used to check if it's a directory */
         cp = last_char_is(file_header->name, '/');
 
         if (archive_handle->filter(archive_handle) == EXIT_SUCCESS) {
diff --git a/archival/libunarchive/seek_by_read.c b/archival/libunarchive/seek_by_read.c
index d56f94b21..e46af4842 100644
--- a/archival/libunarchive/seek_by_read.c
+++ b/archival/libunarchive/seek_by_read.c
@@ -13,7 +13,6 @@
  */
 void seek_by_read(const archive_handle_t *archive_handle, const unsigned int jump_size)
 {
-        if (jump_size) {
-                bb_copyfd_size(archive_handle->src_fd, -1, jump_size);
-        }
+        if (jump_size)
+                bb_copyfd_exact_size(archive_handle->src_fd, -1, jump_size);
 }
diff --git a/archival/tar.c b/archival/tar.c
index 7465e881b..ee9007c47 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -452,26 +452,28 @@ static int writeFileToTarball(const char *fileName, struct stat *statbuf,
 
         /* If it was a regular file, write out the body */
         if (inputFileFd >= 0) {
-                off_t readSize = 0;
-
-                /* write the file to the archive */
-                readSize = bb_copyfd_size(inputFileFd, tbInfo->tarFd, statbuf->st_size);
-                /* readSize < 0 means that error was already reported */
-                if (readSize != statbuf->st_size && readSize >= 0) {
-                        /* Deadly. We record size into header first, */
-                        /* and then write out file. If file shrinks in between, */
-                        /* tar will be corrupted. So bail out. */
-                        /* NB: GNU tar 1.16 warns and pads with zeroes */
-                        /* or even seeks back and updates header */
-                        bb_error_msg_and_die("short read from %s, aborting", fileName);
-                }
+                size_t readSize;
+                /* Wwrite the file to the archive. */
+                /* We record size into header first, */
+                /* and then write out file. If file shrinks in between, */
+                /* tar will be corrupted. So we don't allow for that. */
+                /* NB: GNU tar 1.16 warns and pads with zeroes */
+                /* or even seeks back and updates header */
+                bb_copyfd_exact_size(inputFileFd, tbInfo->tarFd, statbuf->st_size);
+                ////off_t readSize;
+                ////readSize = bb_copyfd_size(inputFileFd, tbInfo->tarFd, statbuf->st_size);
+                ////if (readSize != statbuf->st_size && readSize >= 0) {
+                ////    bb_error_msg_and_die("short read from %s, aborting", fileName);
+                ////}
+
                 /* Check that file did not grow in between? */
-                /* if (safe_read(inputFileFd,1) == 1) warn but continue? */
+                /* if (safe_read(inputFileFd, 1) == 1) warn but continue? */
+
                 close(inputFileFd);
 
                 /* Pad the file up to the tar block size */
                 /* (a few tricks here in the name of code size) */
-                readSize = (-(int)readSize) & (TAR_BLOCK_SIZE-1);
+                readSize = (-(int)statbuf->st_size) & (TAR_BLOCK_SIZE-1);
                 memset(bb_common_bufsiz1, 0, readSize);
                 xwrite(tbInfo->tarFd, bb_common_bufsiz1, readSize);
         }
diff --git a/archival/unzip.c b/archival/unzip.c
index f553eefa2..1c03a4c47 100644
--- a/archival/unzip.c
+++ b/archival/unzip.c
@@ -54,9 +54,9 @@ typedef union {
 static void unzip_skip(int fd, off_t skip)
 {
         if (lseek(fd, skip, SEEK_CUR) == (off_t)-1) {
-                if ((errno != ESPIPE) || (bb_copyfd_size(fd, -1, skip) != skip)) {
+                if (errno != ESPIPE)
                         bb_error_msg_and_die("seek failure");
-                }
+                bb_copyfd_exact_size(fd, -1, skip);
         }
 }
 
@@ -75,10 +75,8 @@ static int unzip_extract(zip_header_t *zip_header, int src_fd, int dst_fd)
         if (zip_header->formatted.method == 0) {
                 /* Method 0 - stored (not compressed) */
                 off_t size = zip_header->formatted.ucmpsize;
-                if (size && (bb_copyfd_size(src_fd, dst_fd, size) != size)) {
-                        bb_error_msg_and_die("cannot complete extraction");
-                }
-
+                if (size)
+                        bb_copyfd_exact_size(src_fd, dst_fd, size);
         } else {
                 /* Method 8 - inflate */
                 inflate_init(zip_header->formatted.cmpsize);
@@ -249,8 +247,8 @@ int unzip_main(int argc, char **argv)
                 unzip_skip(src_fd, zip_header.formatted.extra_len);
 
                 if ((verbosity == v_list) && !list_header_done){
-                        printf("  Length     Date   Time    Name\n"
-                               " --------    ----   ----    ----\n");
+                        puts("  Length     Date   Time    Name\n"
+                             " --------    ----   ----    ----");
                         list_header_done = 1;
                 }
 
@@ -274,10 +272,8 @@ int unzip_main(int argc, char **argv)
                                            dst_fn);
                                 total_entries++;
                                 i = 'n';
-
                         } else if (dst_fd == STDOUT_FILENO) { /* Extracting to STDOUT */
                                 i = -1;
-
                         } else if (last_char_is(dst_fn, '/')) { /* Extract directory */
                                 if (stat(dst_fn, &stat_buf) == -1) {
                                         if (errno != ENOENT) {
@@ -298,17 +294,15 @@ int unzip_main(int argc, char **argv)
                                 i = 'n';
 
                         } else {  /* Extract file */
-                        _check_file:
+ _check_file:
                                 if (stat(dst_fn, &stat_buf) == -1) { /* File does not exist */
                                         if (errno != ENOENT) {
                                                 bb_perror_msg_and_die("cannot stat '%s'",dst_fn);
                                         }
                                         i = 'y';
-
                                 } else { /* File already exists */
                                         if (overwrite == o_never) {
                                                 i = 'n';
-
                                         } else if (S_ISREG(stat_buf.st_mode)) { /* File is regular file */
                                                 if (overwrite == o_always) {
                                                         i = 'y';
@@ -319,7 +313,6 @@ int unzip_main(int argc, char **argv)
                                                         }
                                                         i = key_buf[0];
                                                 }
-
                                         } else { /* File is not regular file */
                                                 bb_error_msg_and_die("'%s' exists but is not regular file",dst_fn);
                                         }
@@ -338,7 +331,7 @@ int unzip_main(int argc, char **argv)
                                 printf("  inflating: %s\n", dst_fn);
                         }
                         if (unzip_extract(&zip_header, src_fd, dst_fd)) {
-                            failed = 1;
+                                failed = 1;
                         }
                         if (dst_fd != STDOUT_FILENO) {
                                 /* closing STDOUT is potentially bad for future business */
diff --git a/coreutils/cat.c b/coreutils/cat.c
index d828b86ec..2b7c6035f 100644
--- a/coreutils/cat.c
+++ b/coreutils/cat.c
@@ -25,9 +25,8 @@ int bb_cat(char **argv)
                 if (f) {
                         off_t r = bb_copyfd_eof(fileno(f), STDOUT_FILENO);
                         fclose_if_not_stdin(f);
-                        if (r >= 0) {
+                        if (r >= 0)
                                 continue;
-                        }
                 }
                 retval = EXIT_FAILURE;
         } while (*++argv);
diff --git a/include/libbb.h b/include/libbb.h
index 2fd54e789..2bfeba4e1 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -203,8 +203,13 @@ extern int recursive_action(const char *fileName, int recurse,
 extern int device_open(const char *device, int mode);
 extern int get_console_fd(void);
 extern char *find_block_device(char *path);
-extern off_t bb_copyfd_size(int fd1, int fd2, off_t size);
+/* bb_copyfd_XX print read/write errors and return -1 if they occur */
 extern off_t bb_copyfd_eof(int fd1, int fd2);
+extern off_t bb_copyfd_size(int fd1, int fd2, off_t size);
+extern void bb_copyfd_exact_size(int fd1, int fd2, off_t size);
+/* "short" copy can be detected by return value < size */
+/* this helper yells "short read!" if param is not -1 */
+extern void complain_copyfd_and_die(off_t sz) ATTRIBUTE_NORETURN;
 extern char bb_process_escape_sequence(const char **ptr);
 extern char *bb_get_last_path_component(char *path);
 extern int ndelay_on(int fd);
diff --git a/include/unarchive.h b/include/unarchive.h
index 7de6a63fe..88c00882c 100644
--- a/include/unarchive.h
+++ b/include/unarchive.h
@@ -24,7 +24,7 @@ typedef struct file_headers_s {
 } file_header_t;
 
 typedef struct archive_handle_s {
-        /* define if the header and data component should processed */
+        /* define if the header and data component should be processed */
         char (*filter)(struct archive_handle_s *);
         llist_t *accept;
         /* List of files that have been rejected */
diff --git a/libbb/copyfd.c b/libbb/copyfd.c
index c6b886647..17bf4fbc2 100644
--- a/libbb/copyfd.c
+++ b/libbb/copyfd.c
@@ -39,7 +39,7 @@ static off_t bb_full_fd_action(int src_fd, int dst_fd, off_t size)
 
                 rd = safe_read(src_fd, buffer, size > BUFSIZ ? BUFSIZ : size);
 
-                if (!rd) { /* eof - all done. */
+                if (!rd) { /* eof - all done */
                         status = 0;
                         break;
                 }
@@ -56,22 +56,31 @@ static off_t bb_full_fd_action(int src_fd, int dst_fd, off_t size)
                         }
                 }
                 total += rd;
-                if (status < 0) {
+                if (status < 0) { /* if we aren't copying till EOF... */
                         size -= rd;
                         if (!size) {
-                                status = 0;
+                                /* 'size' bytes copied - all done */
+                                status = 0; 
                                 break;
                         }
                 }
         }
-
-out:
+ out:
         RELEASE_CONFIG_BUFFER(buffer);
-
         return status ? -1 : total;
 }
 
 
+#if 0
+void complain_copyfd_and_die(off_t sz)
+{
+        if (sz != -1)
+                bb_error_msg_and_die("short read");
+        /* if sz == -1, bb_copyfd_XX already complained */
+        exit(xfunc_error_retval);
+}
+#endif
+
 off_t bb_copyfd_size(int fd1, int fd2, off_t size)
 {
         if (size) {
@@ -80,6 +89,17 @@ off_t bb_copyfd_size(int fd1, int fd2, off_t size)
         return 0;
 }
 
+void bb_copyfd_exact_size(int fd1, int fd2, off_t size)
+{
+        off_t sz = bb_copyfd_size(fd1, fd2, size);
+        if (sz == size)
+                return;
+        if (sz != -1)
+                bb_error_msg_and_die("short read");
+        /* if sz == -1, bb_copyfd_XX already complained */
+        exit(xfunc_error_retval);
+}
+
 off_t bb_copyfd_eof(int fd1, int fd2)
 {
         return bb_full_fd_action(fd1, fd2, 0);
diff --git a/networking/ftpgetput.c b/networking/ftpgetput.c
index dff894468..9d054428f 100644
--- a/networking/ftpgetput.c
+++ b/networking/ftpgetput.c
@@ -22,8 +22,8 @@ typedef struct ftp_host_info_s {
         struct sockaddr_in *s_in;
 } ftp_host_info_t;
 
-static char verbose_flag = 0;
-static char do_continue = 0;
+static char verbose_flag;
+static char do_continue;
 
 static int ftpcmd(const char *s1, const char *s2, FILE *stream, char *buf)
 {
@@ -112,7 +112,9 @@ int ftp_receive(ftp_host_info_t *server, FILE *control_stream,
                 const char *local_path, char *server_path)
 {
         char buf[512];
-        off_t filesize = 0;
+/* I think 'filesize' usage here is bogus. Let's see... */
+        //off_t filesize = -1;
+#define filesize ((off_t)-1)
         int fd_data;
         int fd_local = -1;
         off_t beg_range = 0;
@@ -124,11 +126,10 @@ int ftp_receive(ftp_host_info_t *server, FILE *control_stream,
         fd_data = xconnect_ftpdata(server, buf);
 
         if (ftpcmd("SIZE ", server_path, control_stream, buf) == 213) {
-                filesize = BB_STRTOOFF(buf + 4, NULL, 10);
-                if (errno || filesize < 0)
-                        bb_error_msg_and_die("SIZE error: %s", buf + 4);
+                //filesize = BB_STRTOOFF(buf + 4, NULL, 10);
+                //if (errno || filesize < 0)
+                //      bb_error_msg_and_die("SIZE error: %s", buf + 4);
         } else {
-                filesize = -1;
                 do_continue = 0;
         }
 
@@ -154,7 +155,8 @@ int ftp_receive(ftp_host_info_t *server, FILE *control_stream,
                 if (ftpcmd(buf, NULL, control_stream, buf) != 350) {
                         do_continue = 0;
                 } else {
-                        filesize -= beg_range;
+                        //if (filesize != -1)
+                        //      filesize -= beg_range;
                 }
         }
 
@@ -173,11 +175,11 @@ int ftp_receive(ftp_host_info_t *server, FILE *control_stream,
 
         /* Copy the file */
         if (filesize != -1) {
-                if (-1 == bb_copyfd_size(fd_data, fd_local, filesize))
-                        exit(EXIT_FAILURE);
+                if (bb_copyfd_size(fd_data, fd_local, filesize) == -1)
+                        return EXIT_FAILURE;
         } else {
-                if (-1 == bb_copyfd_eof(fd_data, fd_local))
-                        exit(EXIT_FAILURE);
+                if (bb_copyfd_eof(fd_data, fd_local) == -1)
+                        return EXIT_FAILURE;
         }
 
         /* close it all down */
@@ -277,14 +279,11 @@ int ftpgetput_main(int argc, char **argv)
         /* content-length of the file */
         unsigned opt;
         char *port = "ftp";
-
         /* socket to ftp server */
         FILE *control_stream;
         struct sockaddr_in s_in;
-
         /* continue a prev transfer (-c) */
         ftp_host_info_t *server;
-
         int (*ftp_action)(ftp_host_info_t *, FILE *, const char *, char *) = NULL;
 
         /* Check to see if the command is ftpget or ftput */