From 4053ba7cfb5255c0e6832781aebc909666b2e984 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 27 Feb 2021 14:39:06 -0800 Subject: [PATCH] Pacify rtapelib.c for GCC 10 and fix a bug or two MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch assumes C99, but that’s OK nowadays. * lib/rtapelib.c: Include verify.h. (rmt_open__): Tell GCC that remote_file must be nonnull. (rmt_read__, rmt_write__, rmt_lseek__, rmt_ioctl__): Properly size outgoing command buffers instead of guessing 64. Simplify by using C99 printf formats like %zu, as that’s a safe assumption nowadays. (rmt_ioctl__): 3rd arg is now void * not char *, to pacify gcc -Wcast-align. Fix unlikely bug with short reads: ARGUMENT was being incremented, whereas later code wanted the original ARGUMENT. * paxlib.modules: Add ‘verify’. --- lib/rmt.h | 2 +- lib/rtapelib.c | 64 ++++++++++++++++++++------------------------------ paxlib.modules | 1 + 3 files changed, 28 insertions(+), 39 deletions(-) --- a/lib/rmt.h +++ b/lib/rmt.h @@ -25,7 +25,7 @@ int rmt_close__ (int); size_t rmt_read__ (int, char *, size_t); size_t rmt_write__ (int, char *, size_t); off_t rmt_lseek__ (int, off_t, int); -int rmt_ioctl__ (int, int, char *); +int rmt_ioctl__ (int, int, void *); extern bool force_local_option; --- a/lib/rtapelib.c +++ b/lib/rtapelib.c @@ -37,6 +37,7 @@ #include #include +#include /* Try hard to get EOPNOTSUPP defined. 486/ISC has it in net/errno.h, 3B2/SVR3 has it in sys/inet.h. Otherwise, like on MSDOS, use EINVAL. */ @@ -424,6 +425,8 @@ rmt_open__ (const char *file_name, int o } } + assume (remote_file); + /* FIXME: Should somewhat validate the decoding, here. */ if (gethostbyname (remote_host) == NULL) error (EXIT_ON_EXEC_ERROR, 0, _("Cannot connect to %s: resolve failed"), @@ -567,12 +570,12 @@ rmt_close__ (int handle) size_t rmt_read__ (int handle, char *buffer, size_t length) { - char command_buffer[COMMAND_BUFFER_SIZE]; + char command_buffer[sizeof "R\n" + INT_STRLEN_BOUND (size_t)]; size_t status; size_t rlen; size_t counter; - sprintf (command_buffer, "R%lu\n", (unsigned long) length); + sprintf (command_buffer, "R%zu\n", length); if (do_command (handle, command_buffer) == -1 || (status = get_status (handle)) == SAFE_READ_ERROR || status > length) @@ -596,11 +599,11 @@ rmt_read__ (int handle, char *buffer, si size_t rmt_write__ (int handle, char *buffer, size_t length) { - char command_buffer[COMMAND_BUFFER_SIZE]; + char command_buffer[sizeof "W\n" + INT_STRLEN_BOUND (size_t)]; RETSIGTYPE (*pipe_handler) (int); size_t written; - sprintf (command_buffer, "W%lu\n", (unsigned long) length); + sprintf (command_buffer, "W%zu\n", length); if (do_command (handle, command_buffer) == -1) return 0; @@ -628,17 +631,7 @@ rmt_write__ (int handle, char *buffer, s off_t rmt_lseek__ (int handle, off_t offset, int whence) { - char command_buffer[COMMAND_BUFFER_SIZE]; - char operand_buffer[UINTMAX_STRSIZE_BOUND]; - uintmax_t u = offset < 0 ? - (uintmax_t) offset : (uintmax_t) offset; - char *p = operand_buffer + sizeof operand_buffer; - - *--p = 0; - do - *--p = '0' + (int) (u % 10); - while ((u /= 10) != 0); - if (offset < 0) - *--p = '-'; + char command_buffer[sizeof "L\n0\n" + INT_STRLEN_BOUND (+offset)]; switch (whence) { @@ -648,7 +641,8 @@ rmt_lseek__ (int handle, off_t offset, i default: abort (); } - sprintf (command_buffer, "L%s\n%d\n", p, whence); + intmax_t off = offset; + sprintf (command_buffer, "L%jd\n%d\n", off, whence); if (do_command (handle, command_buffer) == -1) return -1; @@ -659,7 +653,7 @@ rmt_lseek__ (int handle, off_t offset, i /* Perform a raw tape operation on remote tape connection HANDLE. Return the results of the ioctl, or -1 on error. */ int -rmt_ioctl__ (int handle, int operation, char *argument) +rmt_ioctl__ (int handle, int operation, void *argument) { switch (operation) { @@ -670,24 +664,16 @@ rmt_ioctl__ (int handle, int operation, #ifdef MTIOCTOP case MTIOCTOP: { - char command_buffer[COMMAND_BUFFER_SIZE]; - char operand_buffer[UINTMAX_STRSIZE_BOUND]; - uintmax_t u = (((struct mtop *) argument)->mt_count < 0 - ? - (uintmax_t) ((struct mtop *) argument)->mt_count - : (uintmax_t) ((struct mtop *) argument)->mt_count); - char *p = operand_buffer + sizeof operand_buffer; - - *--p = 0; - do - *--p = '0' + (int) (u % 10); - while ((u /= 10) != 0); - if (((struct mtop *) argument)->mt_count < 0) - *--p = '-'; + struct mtop *mtop = argument; + enum { oplen = INT_STRLEN_BOUND (+mtop->mt_op) }; + enum { countlen = INT_STRLEN_BOUND (+mtop->mt_count) }; + char command_buffer[sizeof "I\n\n" + oplen + countlen]; /* MTIOCTOP is the easy one. Nothing is transferred in binary. */ - sprintf (command_buffer, "I%d\n%s\n", - ((struct mtop *) argument)->mt_op, p); + verify (EXPR_SIGNED (mtop->mt_count)); + intmax_t count = mtop->mt_count; + sprintf (command_buffer, "I%d\n%jd\n", mtop->mt_op, count); if (do_command (handle, command_buffer) == -1) return -1; @@ -717,9 +703,9 @@ rmt_ioctl__ (int handle, int operation, return -1; } - for (; status > 0; status -= counter, argument += counter) + for (char *p = argument; status > 0; status -= counter, p += counter) { - counter = safe_read (READ_SIDE (handle), argument, status); + counter = safe_read (READ_SIDE (handle), p, status); if (counter == SAFE_READ_ERROR || counter == 0) { _rmt_shutdown (handle, EIO); @@ -732,15 +718,17 @@ rmt_ioctl__ (int handle, int operation, than 256, we will assume that the bytes are swapped and go through and reverse all the bytes. */ - if (((struct mtget *) argument)->MTIO_CHECK_FIELD < 256) + struct mtget *mtget = argument; + if (mtget->MTIO_CHECK_FIELD < 256) return 0; + char *buf = argument; for (counter = 0; counter < status; counter += 2) { - char copy = argument[counter]; + char copy = buf[counter]; - argument[counter] = argument[counter + 1]; - argument[counter + 1] = copy; + buf[counter] = buf[counter + 1]; + buf[counter + 1] = copy; } return 0;