maint: simplify memory alignment

Use the new Gnulib modules alignalloc and xalignalloc
to simplify some memory allocation.
Also, fix some unlikely integer overflow problems.
* bootstrap.conf (gnulib_modules): Add alignalloc, xalignalloc.
* src/cat.c, src/copy.c, src/dd.c, src/shred.c, src/split.c:
Include alignalloc.h.
* src/cat.c (main):
* src/copy.c (copy_reg):
* src/dd.c (alloc_ibuf, alloc_obuf):
* src/shred.c (dopass):
* src/split.c (main):
Use alignalloc/xalignalloc/alignfree instead of doing page
alignment by hand.
* src/cat.c (main):
Check for integer overflow in page size calculations.
* src/dd.c (INPUT_BLOCK_SLOP, OUTPUT_BLOCK_SLOP, MAX_BLOCKSIZE):
(real_ibuf, real_obuf) [lint]:
Remove; no longer needed.
(cleanup) [lint]:
(scanargs): Simplify.
* src/ioblksize.h (io_blksize): Do not allow blocksizes largest
than the largest power of two that fits in idx_t and size_t.
* src/shred.c (PAGE_ALIGN_SLOP, PATTERNBUF_SIZE): Remove.

diff --git a/bootstrap.conf b/bootstrap.conf
index 48f3551074f186a6f02af17b79803743968ca583..5ca56f91772f4ec1812f8ece41eed18c3f2b9f7e 100644 (file)
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -26,6 +26,7 @@ avoided_gnulib_modules='
 gnulib_modules="
   $avoided_gnulib_modules
   acl
+  alignalloc
   alignof
   alloca
   announce-gen
@@ -294,6 +295,7 @@ gnulib_modules="
   winsz-ioctl
   winsz-termios
   write-any-file
+  xalignalloc
   xalloc
   xbinary-io
   xdectoint

diff --git a/src/cat.c b/src/cat.c
index 3b9ba5356402b681d8933c7dc153213d5cdc532e..1d6f7fbff97143c8267e708ac937913c6b373faa 100644 (file)
--- a/src/cat.c
+++ b/src/cat.c
@@ -33,6 +33,7 @@
 #include <sys/ioctl.h>
 
 #include "system.h"
+#include "alignalloc.h"
 #include "idx.h"
 #include "ioblksize.h"
 #include "die.h"
@@ -690,16 +691,17 @@ main (int argc, char **argv)
              || show_tabs || squeeze_blank))
         {
           insize = MAX (insize, outsize);
-          inbuf = ximalloc (insize + page_size - 1);
+          inbuf = xalignalloc (page_size, insize);
 
-          ok &= simple_cat (ptr_align (inbuf, page_size), insize);
+          ok &= simple_cat (inbuf, insize);
         }
       else
         {
-          inbuf = ximalloc (insize + 1 + page_size - 1);
+          /* Allocate, with an extra byte for a newline sentinel.  */
+          inbuf = xalignalloc (page_size, insize + 1);
 
           /* Why are
-             (OUTSIZE - 1 + INSIZE * 4 + LINE_COUNTER_BUF_LEN + PAGE_SIZE - 1)
+             (OUTSIZE - 1 + INSIZE * 4 + LINE_COUNTER_BUF_LEN)
              bytes allocated for the output buffer?
 
              A test whether output needs to be written is done when the input
@@ -717,21 +719,23 @@ main (int argc, char **argv)
              positions.
 
              Align the output buffer to a page size boundary, for efficiency
-             on some paging implementations, so add PAGE_SIZE - 1 bytes to the
-             request to make room for the alignment.  */
+             on some paging implementations.  */
 
-          char *outbuf = ximalloc (outsize - 1 + insize * 4
-                                   + LINE_COUNTER_BUF_LEN + page_size - 1);
+          idx_t bufsize;
+          if (INT_MULTIPLY_WRAPV (insize, 4, &bufsize)
+              || INT_ADD_WRAPV (bufsize, outsize, &bufsize)
+              || INT_ADD_WRAPV (bufsize, LINE_COUNTER_BUF_LEN - 1, &bufsize))
+            xalloc_die ();
+          char *outbuf = xalignalloc (page_size, bufsize);
 
-          ok &= cat (ptr_align (inbuf, page_size), insize,
-                     ptr_align (outbuf, page_size), outsize, show_nonprinting,
+          ok &= cat (inbuf, insize, outbuf, outsize, show_nonprinting,
                      show_tabs, number, number_nonblank, show_ends,
                      squeeze_blank);
 
-          free (outbuf);
+          alignfree (outbuf);
         }
 
-      free (inbuf);
+      alignfree (inbuf);
 
     contin:
       if (!reading_stdin && close (input_desc) < 0)

diff --git a/src/copy.c b/src/copy.c
index b2e3cb1f76efc7a9b60684f63689f49b48c224e4..4a7d9b5d9cbc7e4b96f3682d45bb13f2856ead75 100644 (file)
--- a/src/copy.c
+++ b/src/copy.c
@@ -32,6 +32,7 @@
 
 #include "system.h"
 #include "acl.h"
+#include "alignalloc.h"
 #include "backupfile.h"
 #include "buffer-lcm.h"
 #include "canonicalize.h"
@@ -229,8 +230,6 @@ create_hole (int fd, char const *name, bool punch_holes, off_t size)
    Return true upon successful completion;
    print a diagnostic and return false upon error.
    Note that for best results, BUF should be "well"-aligned.
-   BUF must have sizeof(uintptr_t)-1 bytes of additional space
-   beyond BUF[BUF_SIZE - 1].
    Set *LAST_WRITE_MADE_HOLE to true if the final operation on
    DEST_FD introduced a hole.  Set *TOTAL_N_READ to the number of
    bytes read.  */
@@ -1076,8 +1075,7 @@ copy_reg (char const *src_name, char const *dst_name,
           mode_t dst_mode, mode_t omitted_permissions, bool *new_dst,
           struct stat const *src_sb)
 {
-  char *buf;
-  char *buf_alloc = NULL;
+  char *buf = NULL;
   int dest_desc;
   int dest_errno;
   int source_desc;
@@ -1292,7 +1290,6 @@ copy_reg (char const *src_name, char const *dst_name,
   if (data_copy_required)
     {
       /* Choose a suitable buffer size; it may be adjusted later.  */
-      size_t buf_alignment = getpagesize ();
       size_t buf_size = io_blksize (sb);
       size_t hole_size = ST_BLKSIZE (sb);
 
@@ -1319,7 +1316,7 @@ copy_reg (char const *src_name, char const *dst_name,
         {
           /* Compute the least common multiple of the input and output
              buffer sizes, adjusting for outlandish values.  */
-          size_t blcm_max = MIN (SIZE_MAX, SSIZE_MAX) - buf_alignment;
+          size_t blcm_max = MIN (SIZE_MAX, SSIZE_MAX);
           size_t blcm = buffer_lcm (io_blksize (src_open_sb), buf_size,
                                     blcm_max);
 
@@ -1337,8 +1334,7 @@ copy_reg (char const *src_name, char const *dst_name,
             buf_size = blcm;
         }
 
-      buf_alloc = xmalloc (buf_size + buf_alignment);
-      buf = ptr_align (buf_alloc, buf_alignment);
+      buf = xalignalloc (getpagesize (), buf_size);
 
       off_t n_read;
       bool wrote_hole_at_eof = false;
@@ -1457,7 +1453,7 @@ close_src_desc:
       return_val = false;
     }
 
-  free (buf_alloc);
+  alignfree (buf);
   return return_val;
 }
 

diff --git a/src/dd.c b/src/dd.c
index b52ef094800bbc13e2f7de91f4efd6f234f1e9b4..a6a3708f16707c7c4e60313edb4d402238811c9d 100644 (file)
--- a/src/dd.c
+++ b/src/dd.c
@@ -22,6 +22,7 @@
 #include <signal.h>
 
 #include "system.h"
+#include "alignalloc.h"
 #include "close-stream.h"
 #include "die.h"
 #include "error.h"
@@ -89,20 +90,6 @@
 /* Default input and output blocksize. */
 #define DEFAULT_BLOCKSIZE 512
 
-/* How many bytes to add to the input and output block sizes before invoking
-   malloc.  See dd_copy for details.  INPUT_BLOCK_SLOP must be no less than
-   OUTPUT_BLOCK_SLOP, and has one more byte because of swab_buffer.  */
-#define INPUT_BLOCK_SLOP page_size
-#define OUTPUT_BLOCK_SLOP (page_size - 1)
-
-/* Maximum blocksize for the given SLOP.
-   Keep it smaller than MIN (IDX_MAX, SIZE_MAX) - SLOP, so that we can
-   allocate buffers that size.  Keep it smaller than SSIZE_MAX, for
-   the benefit of system calls like "read".  And keep it smaller than
-   OFF_T_MAX, for the benefit of the large-offset seek code.  */
-#define MAX_BLOCKSIZE(slop) MIN (MIN (IDX_MAX, SIZE_MAX) - (slop), \
-                                 MIN (SSIZE_MAX, OFF_T_MAX))
-
 /* Conversions bit masks. */
 enum
   {
@@ -239,12 +226,6 @@ static intmax_t r_truncate = 0;
 static char newline_character = '\n';
 static char space_character = ' ';
 
-#ifdef lint
-/* Memory blocks allocated for I/O buffers and surrounding areas.  */
-static char *real_ibuf;
-static char *real_obuf;
-#endif
-
 /* I/O buffers.  */
 static char *ibuf;
 static char *obuf;
@@ -695,9 +676,9 @@ alloc_ibuf (void)
   if (ibuf)
     return;
 
-  /* Ensure the input buffer is page aligned.  */
-  char *buf = malloc (input_blocksize + INPUT_BLOCK_SLOP);
-  if (!buf)
+  bool extra_byte_for_swab = !!(conversions_mask & C_SWAB);
+  ibuf = alignalloc (page_size, input_blocksize + extra_byte_for_swab);
+  if (!ibuf)
     {
       char hbuf[LONGEST_HUMAN_READABLE + 1];
       die (EXIT_FAILURE, 0,
@@ -706,10 +687,6 @@ alloc_ibuf (void)
            human_readable (input_blocksize, hbuf,
                            human_opts | human_base_1024, 1, 1));
     }
-#ifdef lint
-  real_ibuf = buf;
-#endif
-  ibuf = ptr_align (buf, page_size);
 }
 
 /* Ensure output buffer OBUF is allocated/initialized.  */
@@ -722,9 +699,8 @@ alloc_obuf (void)
 
   if (conversions_mask & C_TWOBUFS)
     {
-      /* Page-align the output buffer, too.  */
-      char *buf = malloc (output_blocksize + OUTPUT_BLOCK_SLOP);
-      if (!buf)
+      obuf = alignalloc (page_size, output_blocksize);
+      if (!obuf)
         {
           char hbuf[LONGEST_HUMAN_READABLE + 1];
           die (EXIT_FAILURE, 0,
@@ -734,10 +710,6 @@ alloc_obuf (void)
                human_readable (output_blocksize, hbuf,
                                human_opts | human_base_1024, 1, 1));
         }
-#ifdef lint
-      real_obuf = buf;
-#endif
-      obuf = ptr_align (buf, page_size);
     }
   else
     {
@@ -966,10 +938,9 @@ static void
 cleanup (void)
 {
 #ifdef lint
-  free (real_ibuf);
-  free (real_obuf);
-  real_ibuf = NULL;
-  real_obuf = NULL;
+  if (ibuf != obuf)
+    alignfree (ibuf);
+  alignfree (obuf);
 #endif
 
   if (iclose (STDIN_FILENO) != 0)
@@ -1552,22 +1523,29 @@ scanargs (int argc, char *const *argv)
           intmax_t n_max = INTMAX_MAX;
           idx_t *converted_idx = NULL;
 
+          /* Maximum blocksize.  Keep it smaller than IDX_MAX, so that
+             it fits into blocksize vars even if 1 is added for conv=swab.
+             Do not exceed SSIZE_MAX, for the benefit of system calls
+             like "read".  And do not exceed OFF_T_MAX, for the
+             benefit of the large-offset seek code.  */
+          idx_t max_blocksize = MIN (IDX_MAX - 1, MIN (SSIZE_MAX, OFF_T_MAX));
+
           if (operand_is (name, "ibs"))
             {
               n_min = 1;
-              n_max = MAX_BLOCKSIZE (INPUT_BLOCK_SLOP);
+              n_max = max_blocksize;
               converted_idx = &input_blocksize;
             }
           else if (operand_is (name, "obs"))
             {
               n_min = 1;
-              n_max = MAX_BLOCKSIZE (OUTPUT_BLOCK_SLOP);
+              n_max = max_blocksize;
               converted_idx = &output_blocksize;
             }
           else if (operand_is (name, "bs"))
             {
               n_min = 1;
-              n_max = MAX_BLOCKSIZE (INPUT_BLOCK_SLOP);
+              n_max = max_blocksize;
               converted_idx = &blocksize;
             }
           else if (operand_is (name, "cbs"))

diff --git a/src/ioblksize.h b/src/ioblksize.h
index 8f8cd1fc270ca5a4b6a985a5841846a219d50cf3..8bd18ba05185e805f9a61293360d0cf82da2bcc0 100644 (file)
--- a/src/ioblksize.h
+++ b/src/ioblksize.h
@@ -16,7 +16,8 @@
 
 /* Include this file _after_ system headers if possible.  */
 
-/* sys/stat.h will already have been included by system.h. */
+/* sys/stat.h and minmax.h will already have been included by system.h. */
+#include "idx.h"
 #include "stat-size.h"
 
 
@@ -71,8 +72,11 @@
    and default to io_blksize() if not.
  */
 enum { IO_BUFSIZE = 128 * 1024 };
-static inline size_t
+static inline idx_t
 io_blksize (struct stat sb)
 {
-  return MAX (IO_BUFSIZE, ST_BLKSIZE (sb));
+  /* Don’t go above the largest power of two that fits in idx_t and size_t,
+     as that is asking for trouble.  */
+  return MIN (MIN (IDX_MAX, SIZE_MAX) / 2 + 1,
+              MAX (IO_BUFSIZE, ST_BLKSIZE (sb)));
 }

diff --git a/src/shred.c b/src/shred.c
index 6e36b39e41918d4d9cd18072b66219b7afd6b046..e886763807d30c4919b2035849814a5385b7c965 100644 (file)
--- a/src/shred.c
+++ b/src/shred.c
@@ -85,6 +85,7 @@
 #endif
 
 #include "system.h"
+#include "alignalloc.h"
 #include "argmatch.h"
 #include "xdectoint.h"
 #include "die.h"
@@ -412,11 +413,8 @@ dopass (int fd, struct stat const *st, char const *qname, off_t *sizep,
   verify (PERIODIC_OUTPUT_SIZE % 3 == 0);
   size_t output_size = periodic_pattern (type)
                        ? PERIODIC_OUTPUT_SIZE : NONPERIODIC_OUTPUT_SIZE;
-#define PAGE_ALIGN_SLOP (page_size - 1)                /* So directio works */
 #define FILLPATTERN_SIZE (((output_size + 2) / 3) * 3) /* Multiple of 3 */
-#define PATTERNBUF_SIZE (PAGE_ALIGN_SLOP + FILLPATTERN_SIZE)
-  void *fill_pattern_mem = xmalloc (PATTERNBUF_SIZE);
-  unsigned char *pbuf = ptr_align (fill_pattern_mem, page_size);
+  unsigned char *pbuf = xalignalloc (page_size, FILLPATTERN_SIZE);
 
   char pass_string[PASS_NAME_SIZE];    /* Name of current pass */
   bool write_error = false;
@@ -620,7 +618,7 @@ dopass (int fd, struct stat const *st, char const *qname, off_t *sizep,
     }
 
 free_pattern_mem:
-  free (fill_pattern_mem);
+  alignfree (pbuf);
 
   return other_error ? -1 : write_error;
 }

diff --git a/src/split.c b/src/split.c
index b320c22634c0480f3fa619daf4af74a941126295..533c22f9f3823d7f154c64def1d6cef84e609ddc 100644 (file)
--- a/src/split.c
+++ b/src/split.c
@@ -29,6 +29,7 @@
 #include <sys/wait.h>
 
 #include "system.h"
+#include "alignalloc.h"
 #include "die.h"
 #include "error.h"
 #include "fd-reopen.h"
@@ -1300,7 +1301,7 @@ int
 main (int argc, char **argv)
 {
   enum Split_type split_type = type_undef;
-  size_t in_blk_size = 0;      /* optimal block size of input file device */
+  idx_t in_blk_size = 0;       /* optimal block size of input file device */
   size_t page_size = getpagesize ();
   uintmax_t k_units = 0;
   uintmax_t n_units = 0;
@@ -1503,7 +1504,7 @@ main (int argc, char **argv)
           break;
 
         case IO_BLKSIZE_OPTION:
-          in_blk_size = xdectoumax (optarg, 1, SIZE_MAX - page_size,
+          in_blk_size = xdectoumax (optarg, 1, MIN (IDX_MAX, SIZE_MAX) - 1,
                                     multipliers, _("invalid IO block size"), 0);
           break;
 
@@ -1585,8 +1586,7 @@ main (int argc, char **argv)
   if (! specified_buf_size)
     in_blk_size = io_blksize (in_stat_buf);
 
-  void *b = xmalloc (in_blk_size + 1 + page_size - 1);
-  char *buf = ptr_align (b, page_size);
+  char *buf = xalignalloc (page_size, in_blk_size + 1);
   size_t initial_read = SIZE_MAX;
 
   if (split_type == type_chunk_bytes || split_type == type_chunk_lines)
@@ -1661,7 +1661,7 @@ main (int argc, char **argv)
       abort ();
     }
 
-  IF_LINT (free (b));
+  IF_LINT (alignfree (buf));
 
   if (close (STDIN_FILENO) != 0)
     die (EXIT_FAILURE, errno, "%s", quotef (infile));