GraphicsMagick: _MagickReallocateResourceLimitedMemory(): Remove...

changeset 577e84466569 in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=577e84466569
summary: _MagickReallocateResourceLimitedMemory(): Remove size limit from realloc binary expansion.  Fix reallocs tally counts.

diffstat:

 ChangeLog          |   7 +++++++
 coders/gif.c       |   3 +--
 coders/meta.c      |   4 ++--
 coders/miff.c      |   6 +++---
 coders/mpc.c       |   6 +++---
 coders/pdb.c       |   3 +--
 coders/pnm.c       |   3 +--
 coders/xpm.c       |   4 ++--
 magick/memory.c    |  51 ++++++++++++++++++++++++++++++++++++++++-----------
 www/Changelog.html |   5 +++++
 10 files changed, 65 insertions(+), 27 deletions(-)

diffs (272 lines):

diff -r e0e762ead892 -r 577e84466569 ChangeLog
--- a/ChangeLog	Wed Dec 29 11:17:33 2021 -0600
+++ b/ChangeLog	Wed Dec 29 15:09:31 2021 -0600
@@ -1,5 +1,12 @@
 2021-12-29  Bob Friesenhahn  <[email protected]>
 
+        * coders/{gif.c, meta.c, miff.c, mpc.c, pdb.c, pnm.c xpm.c}: Rely
+        on _MagickReallocateResourceLimitedMemory() to extend memory
+        allocation.
+
+        * magick/memory.c (_MagickReallocateResourceLimitedMemory): Remove
+        size limit from realloc binary expansion.  Fix reallocs tally counts.
+
         * magick/memory-private.h(_MagickReallocateResourceLimitedMemory)
         : Remove use of GCC/Clang '__attribute_malloc__' since it is not
         appropriate for this function.
diff -r e0e762ead892 -r 577e84466569 coders/gif.c
--- a/coders/gif.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/coders/gif.c	Wed Dec 29 15:09:31 2021 -0600
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2020 GraphicsMagick Group
+% Copyright (C) 2003-2021 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 % Copyright 1991-1999 E. I. du Pont de Nemours and Company
 %
@@ -993,7 +993,6 @@
                   {
                     char *comments_new;
                     allocation_length=allocation_length+count+1;
-                    MagickRoundUpStringLength(allocation_length);
                     comments_new=MagickReallocateResourceLimitedMemory(char *,
                                                                        comments,
                                                                        allocation_length);
diff -r e0e762ead892 -r 577e84466569 coders/meta.c
--- a/coders/meta.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/coders/meta.c	Wed Dec 29 15:09:31 2021 -0600
@@ -275,7 +275,7 @@
           tlen;
 
         tlen=(size_t) (q-p);
-        len<<=1;
+        len += MaxTextExtent;
         new_p=MagickReallocateResourceLimitedMemory(unsigned char *,p,(len+2));
         if (new_p == (unsigned char *) NULL)
           {
@@ -574,7 +574,7 @@
           tlen;
 
         tlen=(size_t) (q-p);
-        len<<=1;
+        len += MaxTextExtent;
         new_p=MagickReallocateResourceLimitedMemory(unsigned char *,p,(len+2));
         if (new_p == (unsigned char *) NULL)
           {
diff -r e0e762ead892 -r 577e84466569 coders/miff.c
--- a/coders/miff.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/coders/miff.c	Wed Dec 29 15:09:31 2021 -0600
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2020 GraphicsMagick Group
+% Copyright (C) 2003-2021 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 % Copyright 1991-1999 E. I. du Pont de Nemours and Company
 %
@@ -909,7 +909,7 @@
                   *new_comment;
 
                 *p='\0';
-                comment_length<<=1;
+                comment_length += MaxTextExtent;
                 new_comment=MagickReallocateResourceLimitedMemory(char *,comment,comment_length);
                 if (new_comment == (char *) NULL)
                   {
@@ -992,7 +992,7 @@
                       *new_values;
 
                     *p='\0';
-                    values_length<<=1;
+                    values_length += MaxTextExtent;
                     new_values=MagickReallocateResourceLimitedMemory(char *,values,values_length);
                     if (new_values == (char *) NULL)
                       {
diff -r e0e762ead892 -r 577e84466569 coders/mpc.c
--- a/coders/mpc.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/coders/mpc.c	Wed Dec 29 15:09:31 2021 -0600
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2020 GraphicsMagick Group
+% Copyright (C) 2003-2021 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 %
 % This program is covered by multiple licenses, which are described in
@@ -278,7 +278,7 @@
                   *new_comment;
 
                 *p='\0';
-                comment_length<<=1;
+                comment_length += MaxTextExtent;
                 new_comment=MagickReallocateResourceLimitedMemory(char *,comment,comment_length);
                 if (new_comment == (char *) NULL)
                   {
@@ -361,7 +361,7 @@
                       *new_values;
 
                     *p='\0';
-                    values_length<<=1;
+                    values_length += MaxTextExtent;
                     new_values=MagickReallocateResourceLimitedMemory(char *,values,values_length);
                     if (new_values == (char *) NULL)
                       {
diff -r e0e762ead892 -r 577e84466569 coders/pdb.c
--- a/coders/pdb.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/coders/pdb.c	Wed Dec 29 15:09:31 2021 -0600
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2020 GraphicsMagick Group
+% Copyright (C) 2003-2021 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 %
 % This program is covered by multiple licenses, which are described in
@@ -689,7 +689,6 @@
                   char
                     *new_comment;
 
-                  length<<=1;
                   length+=MaxTextExtent;
                   new_comment=MagickReallocateResourceLimitedMemory(char *,comment,length+1);
                   if (new_comment == (char *) NULL)
diff -r e0e762ead892 -r 577e84466569 coders/pnm.c
--- a/coders/pnm.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/coders/pnm.c	Wed Dec 29 15:09:31 2021 -0600
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003-2020 GraphicsMagick Group
+% Copyright (C) 2003-2021 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 % Copyright 1991-1999 E. I. du Pont de Nemours and Company
 %
@@ -194,7 +194,6 @@
                   *new_comment;
 
                 text_length=(size_t) (p-comment);
-                length<<=1;
                 length+=MaxTextExtent;
                 new_comment=MagickReallocateResourceLimitedMemory(char *,comment,length+sizeof(P7Comment));
                 if (new_comment == (char *) NULL)
diff -r e0e762ead892 -r 577e84466569 coders/xpm.c
--- a/coders/xpm.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/coders/xpm.c	Wed Dec 29 15:09:31 2021 -0600
@@ -296,7 +296,7 @@
   /*
     Read XPM file.
   */
-  length=MaxTextExtent;
+  length=MaxTextExtent*4;
   xpm_buffer=MagickAllocateResourceLimitedMemory(char *,length);
   if (xpm_buffer != (char *) NULL)
     {
@@ -313,7 +313,7 @@
           p+=strlen(p);
           if (((size_t) (p-xpm_buffer)+MaxTextExtent+1) < length)
             continue;
-          length<<=1;
+          length += MaxTextExtent*4;
           new_xpm_buffer=MagickReallocateResourceLimitedMemory(char *,xpm_buffer,length);
           if (new_xpm_buffer == (char *) NULL)
             {
diff -r e0e762ead892 -r 577e84466569 magick/memory.c
--- a/magick/memory.c	Wed Dec 29 11:17:33 2021 -0600
+++ b/magick/memory.c	Wed Dec 29 15:09:31 2021 -0600
@@ -634,13 +634,19 @@
 #define TraceMagickAccessMemoryResource_T(operation,memory_resource)    \
   fprintf(stderr,__FILE__ ":%d - %s memory_resource: memory=%p (user %p)," \
           " alloc_size=%zu,"                                            \
-          " alloc_size_real=%zu\n",                                     \
+          " alloc_size_real=%zu,"                                       \
+          " num_realloc=%zu,"                                           \
+          " num_realloc_moves=%zu,"                                     \
+          " realloc_octets_moved=%zu\n",                                \
           __LINE__,                                                     \
           operation,                                                    \
           (memory_resource)->memory,                                    \
           (memory_resource)->memory ? UserLandPointerGivenBaseAlloc((memory_resource)->memory) : 0, \
           (memory_resource)->alloc_size,                                \
-          (memory_resource)->alloc_size_real);
+          (memory_resource)->alloc_size_real,                           \
+          (memory_resource)->num_realloc,                               \
+          (memory_resource)->num_realloc_moves,                         \
+          (memory_resource)->realloc_octets_moved);
 #else
 #define TraceMagickAccessMemoryResource_T(operation,memory_resource) ;
 #endif
@@ -653,6 +659,23 @@
 static void _MagickFreeResourceLimitedMemory_T(MagickMemoryResource_T *memory_resource)
 {
   TraceMagickAccessMemoryResource_T("FREE",memory_resource);
+#if defined(MAGICK_MEMORY_LOG_REALLOC_STATS) && MAGICK_MEMORY_LOG_REALLOC_STATS
+  if (memory_resource->num_realloc > 0)
+    {
+      fprintf(stderr,
+              "FreeResourceLimitedMemory:"
+              " alloc_size=%zu,"
+              " alloc_size_real=%zu,"
+              " num_realloc=%zu,"
+              " num_realloc_moves=%zu,"
+              " realloc_octets_moved=%zu\n",
+              (memory_resource)->alloc_size,
+              (memory_resource)->alloc_size_real+sizeof(MagickMemoryResource_T),
+              (memory_resource)->num_realloc,
+              (memory_resource)->num_realloc_moves,
+              (memory_resource)->realloc_octets_moved);
+    }
+#endif /* defined(MAGICK_MEMORY_LOG_REALLOC_STATS) && MAGICK_MEMORY_LOG_REALLOC_STATS */
   if (memory_resource->memory != 0)
     {
       MagickFree(memory_resource->memory);
@@ -735,10 +758,10 @@
                   size_t realloc_size = new_size+sizeof(MagickMemoryResource_T);
                   /*
                     If this is a realloc, then round up underlying
-                    allocation sizes for small allocations in order to
-                    lessen realloc calls and lessen memory moves.
+                    allocation sizes in order to lessen realloc calls
+                    and lessen memory moves.
                   */
-                  if ((memory_resource.alloc_size_real != 0) && (realloc_size < 131072))
+                  if ((memory_resource.alloc_size_real != 0) /*&& (realloc_size < 131072)*/)
                     {
                       /* realloc_size <<= 1; */
                       MagickRoundUpStringLength(realloc_size);
@@ -751,13 +774,19 @@
                         (void) memset(UserLandPointerGivenBaseAlloc(realloc_memory)+
                                       memory_resource.alloc_size,0,size_diff);
 
-                      /* Tally actual reallocations */
-                      memory_resource.num_realloc++;
-                      /* Tally reallocations which resulted in a memory move */
-                      if (realloc_memory != memory_resource.memory)
+                      /* A realloc has pre-existing memory */
+                      if (memory_resource.alloc_size_real != 0) /* FIXME: memory_resource.alloc_size_real ? */
                         {
-                          memory_resource.num_realloc_moves++;
-                          memory_resource.realloc_octets_moved += memory_resource.alloc_size;
+                          fprintf(stderr,"realloc: %zu -> %zu\n", memory_resource.alloc_size_real, realloc_size);
+                          /* Tally actual reallocations */
+                          memory_resource.num_realloc++;
+                          /* Tally reallocations which resulted in a memory move */
+                          if (realloc_memory != memory_resource.memory)
+                            {
+                              memory_resource.num_realloc_moves++;
+                              memory_resource.realloc_octets_moved +=
+                                memory_resource.alloc_size_real+sizeof(MagickMemoryResource_T);
+                            }
                         }
                       memory_resource.memory = realloc_memory;
                       memory_resource.alloc_size = new_size;
diff -r e0e762ead892 -r 577e84466569 www/Changelog.html
--- a/www/Changelog.html	Wed Dec 29 11:17:33 2021 -0600
+++ b/www/Changelog.html	Wed Dec 29 15:09:31 2021 -0600
@@ -37,6 +37,11 @@
 
 <p>2021-12-29  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
 <blockquote>
+<p>* coders/{gif.c, meta.c, miff.c, mpc.c, pdb.c, pnm.c xpm.c}: Rely
+on _MagickReallocateResourceLimitedMemory() to extend memory
+allocation.</p>
+<p>* magick/memory.c (_MagickReallocateResourceLimitedMemory): Remove
+size limit from realloc binary expansion.  Fix reallocs tally counts.</p>
 <p>* magick/memory-private.h(_MagickReallocateResourceLimitedMemory)
 : Remove use of GCC/Clang '__attribute_malloc__' since it is not
 appropriate for this function.</p>