GraphicsMagick: PerlMagick: Fix/complete implementation of "fill...

GraphicsMagick Commits <[email protected]>
Newsgroups gmane.comp.video.graphicsmagick.cvs
Message-ID <mailman.43604.1640389479.2008.graphicsmagick-commit@lists.sourceforge.net>
changeset 3c58991d3b9b in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=3c58991d3b9b
summary: PerlMagick: Fix/complete implementation of "fill" and "stroke" attributes. Eliminate use of strncpy().

diffstat:

 ChangeLog             |    8 +++
 PerlMagick/Magick.xs  |  103 ++++++++++++++++++++++++++++++++++++++++---------
 www/Changelog.html    |    7 +++
 www/INSTALL-unix.html |    9 +++-
 www/INSTALL-unix.rst  |    8 +++
 www/perl.html         |   10 +--
 www/perl.rst          |    8 +--
 7 files changed, 121 insertions(+), 32 deletions(-)

diffs (389 lines):

diff -r cfb1055afd82 -r 3c58991d3b9b ChangeLog
--- a/ChangeLog	Fri Dec 24 12:46:36 2021 -0600
+++ b/ChangeLog	Fri Dec 24 17:28:54 2021 -0600
@@ -1,5 +1,13 @@
 2021-12-24  Bob Friesenhahn  <[email protected]>
 
+        * PerlMagick/Magick.xs: Fix issue that image fill attribute had
+        its opacity reset to transparent so it could not be usefully set
+        at image scope.  Note that this fix may change results if the fill
+        attribute is not set since then the default will actually be used.
+        Support Get of "fill" and "stroke".  Addresses SourceForge issue
+        #650 "Image attributes not working and not as documented for
+        PerlMagick".  Eliminate use of dangerous strncpy().
+
         * magick/transform.c (TransformImage): Trace crop geometry. Trace
         transform geometry.
         (CropImage): Trace crop geometry. Trace bounding page.
diff -r cfb1055afd82 -r 3c58991d3b9b PerlMagick/Magick.xs
--- a/PerlMagick/Magick.xs	Fri Dec 24 12:46:36 2021 -0600
+++ b/PerlMagick/Magick.xs	Fri Dec 24 17:28:54 2021 -0600
@@ -52,6 +52,7 @@
 #ifdef __cplusplus
 extern "C" {
 #endif
+#define MAGICK_COMMA_DELIM_COLORS 1 /* set to 0 to report color names */
 #define PERL_NO_GET_CONTEXT  /* faster */
 #include "EXTERN.h"
 #include "perl.h"
@@ -1453,10 +1454,10 @@
       if (LocaleCompare(attribute,"filename") == 0)
         {
           if (info)
-            (void) strncpy(info->image_info->filename,SvPV(sval,na),
-              MaxTextExtent-1);
+            (void) MagickStrlCpy(info->image_info->filename,SvPV(sval,na),
+                                 sizeof(info->image_info->filename));
           for ( ; image; image=image->next)
-            (void) strncpy(image->filename,SvPV(sval,na),MaxTextExtent-1);
+            (void) MagickStrlCpy(image->filename,SvPV(sval,na),sizeof(image->filename));
           return;
         }
       if (LocaleCompare(attribute,"file") == 0)
@@ -1613,8 +1614,8 @@
                   info->image_info->filename);
               else
                 for ( ; image; image=image->next)
-                  (void) strncpy(image->magick,info->image_info->magick,
-                    MaxTextExtent-1);
+                  (void) MagickStrlCpy(image->magick,info->image_info->magick,
+                                       sizeof(image->magick));
               DestroyExceptionInfo(&exception);
             }
           return;
@@ -1685,8 +1686,12 @@
       if (LocaleCompare(attribute,"pen") == 0)
         {
           if (info)
-            (void) QueryColorDatabase(SvPV(sval,na),&info->draw_info->fill,
-              image ? &image->exception : &exception);
+            {
+              (void) QueryColorDatabase(SvPV(sval,na),&info->draw_info->fill,
+                                        image ? &image->exception : &exception);
+              (void) QueryColorDatabase(SvPV(sval,na),&info->image_info->pen,
+                                        image ? &image->exception : &exception);
+            }
           return;
         }
       if (LocaleNCompare(attribute,"pixel",5) == 0)
@@ -2411,7 +2416,7 @@
     info=GetPackageInfo(aTHX_ (void *) av,info);
     FormatString(info->image_info->filename,"average-%.*s",MaxTextExtent-9,
       ((p=strrchr(image->filename,'/')) ? p+1 : image->filename));
-    (void) strncpy(image->filename,info->image_info->filename,MaxTextExtent-1);
+    (void) MagickStrlCpy(image->filename,info->image_info->filename,sizeof(image->filename));
     SetImageInfo(info->image_info,SETMAGICK_WRITE,&image->exception);
     SvREFCNT_dec(MY_CXT.error_list);
     MY_CXT.error_jump=NULL;
@@ -3021,7 +3026,7 @@
     info=GetPackageInfo(aTHX_ (void *) av,info);
     FormatString(info->image_info->filename,"average-%.*s",MaxTextExtent-9,
       ((p=strrchr(image->filename,'/')) ? p+1 : image->filename));
-    (void) strncpy(image->filename,info->image_info->filename,MaxTextExtent-1);
+    (void) MagickStrlCpy(image->filename,info->image_info->filename,sizeof(image->filename));
     SetImageInfo(info->image_info,SETMAGICK_WRITE,&image->exception);
     SvREFCNT_dec(MY_CXT.error_list);
     MY_CXT.error_jump=NULL;
@@ -3141,9 +3146,14 @@
             {
               if (!image)
                 break;
+#if MAGICK_COMMA_DELIM_COLORS
               FormatString(color,"%u,%u,%u,%u",image->background_color.red,
                 image->background_color.green,image->background_color.blue,
                 image->background_color.opacity);
+#else
+              (void) QueryColorname(image,&image->background_color,AllCompliance,
+                                    color,&image->exception);
+#endif
               s=newSVpv(color,0);
               PUSHs(s ? sv_2mortal(s) : &sv_undef);
               continue;
@@ -3197,9 +3207,14 @@
             {
               if (!image)
                 break;
+#if MAGICK_COMMA_DELIM_COLORS
               FormatString(color,"%u,%u,%u,%u",image->border_color.red,
                 image->border_color.green,image->border_color.blue,
                 image->border_color.opacity);
+#else
+              (void) QueryColorname(image, &image->border_color,AllCompliance,
+                                    color,&image->exception);
+#endif
               s=newSVpv(color,0);
               PUSHs(s ? sv_2mortal(s) : &sv_undef);
               continue;
@@ -3286,9 +3301,14 @@
               (void) sscanf(attribute,"%*[^[][%d",&j);
               if (j > (long) image->colors)
                 j%=image->colors;
+#if MAGICK_COMMA_DELIM_COLORS
               FormatString(color,"%u,%u,%u,%u",image->colormap[j].red,
                 image->colormap[j].green,image->colormap[j].blue,
                 image->colormap[j].opacity);
+#else
+              (void) QueryColorname(image, &image->colormap[j],AllCompliance,
+                                    color,&image->exception);
+#endif
               s=newSVpv(color,0);
               PUSHs(s ? sv_2mortal(s) : &sv_undef);
               continue;
@@ -3430,6 +3450,23 @@
               PUSHs(s ? sv_2mortal(s) : &sv_undef);
               continue;
             }
+          if (LocaleCompare(attribute,"fill") == 0)
+            {
+              if (!info)
+                break;
+              /* Value duplicated in info->image_info->pen */
+#if MAGICK_COMMA_DELIM_COLORS
+              FormatString(color,"%u,%u,%u,%u",info->draw_info->fill.red,
+                           info->draw_info->fill.green,info->draw_info->fill.blue,
+                           info->draw_info->fill.opacity);
+#else
+              (void) QueryColorname(image, &info->draw_info->fill,AllCompliance,
+                                    color,&image->exception);
+#endif
+              s=newSVpv(color,0);
+              PUSHs(s ? sv_2mortal(s) : &sv_undef);
+              continue;
+            }
           if (LocaleCompare(attribute,"filter") == 0)
             {
               j=image->filter;
@@ -3689,9 +3726,14 @@
             {
               if (!image)
                 break;
+#if MAGICK_COMMA_DELIM_COLORS
               FormatString(color,"%u,%u,%u,%u",image->matte_color.red,
                 image->matte_color.green,image->matte_color.blue,
                 image->matte_color.opacity);
+#else
+              (void) QueryColorname(image, &image->matte_color,AllCompliance,
+                                    color,&image->exception);
+#endif
               s=newSVpv(color,0);
               PUSHs(s ? sv_2mortal(s) : &sv_undef);
               continue;
@@ -3752,8 +3794,13 @@
               (void) sscanf(attribute,"%*[^[][%ld%*[,/]%ld",&x,&y);
               (void) AcquireOnePixelByReference(image,&pixel,(long) (x % image->columns),
                 (long) (y % image->rows),&image->exception);
+#if MAGICK_COMMA_DELIM_COLORS
               FormatString(name,"%u,%u,%u,%u",pixel.red,pixel.green,pixel.blue,
                 pixel.opacity);
+#else
+              (void) QueryColorname(image, &pixel,AllCompliance,
+                                    color,&image->exception);
+#endif
               s=newSVpv(name,0);
               PUSHs(s ? sv_2mortal(s) : &sv_undef);
               continue;
@@ -3888,6 +3935,22 @@
               PUSHs(s ? sv_2mortal(s) : &sv_undef);
               continue;
             }
+          if (LocaleCompare(attribute,"stroke") == 0)
+            {
+              if (!info)
+                break;
+#if MAGICK_COMMA_DELIM_COLORS
+              FormatString(color,"%u,%u,%u,%u",info->draw_info->stroke.red,
+                           info->draw_info->stroke.green,info->draw_info->stroke.blue,
+                           info->draw_info->stroke.opacity);
+#else
+              (void) QueryColorname(image, &info->draw_info->stroke,AllCompliance,
+                                    color,&image->exception);
+#endif
+              s=newSVpv(color,0);
+              PUSHs(s ? sv_2mortal(s) : &sv_undef);
+              continue;
+            }
           MagickError(OptionError,UnrecognizedAttribute,attribute);
           break;
         }
@@ -4134,11 +4197,11 @@
     package_info=ClonePackageInfo(info);
     for (i=2; i < items; i+=2)
       SetAttribute(aTHX_ package_info,image,SvPV(ST(i-1),na),ST(i));
-    (void) strncpy(filename,package_info->image_info->filename,MaxTextExtent-1);
+    (void) MagickStrlCpy(filename,package_info->image_info->filename,sizeof(filename));
     scene=0;
     for (next=image; next; next=next->next)
     {
-      (void) strncpy(next->filename,filename,MaxTextExtent-1);
+      (void) MagickStrlCpy(next->filename,filename,sizeof(next->filename));
       next->scene=scene++;
     }
     SetImageInfo(package_info->image_info,SETMAGICK_WRITE,&image->exception);
@@ -5280,7 +5343,7 @@
 
           draw_info=CloneDrawInfo(info ? info->image_info : (ImageInfo *) NULL,
             info ? info->draw_info : (DrawInfo *) NULL);
-          draw_info->fill.opacity=TransparentOpacity;
+          /* draw_info->fill.opacity=TransparentOpacity; */
           draw_info->stroke.opacity=OpaqueOpacity;
           (void) CloneString(&draw_info->primitive,"Point");
           if (attribute_flag[0] && (argument_list[0].int_reference > 0))
@@ -6711,7 +6774,7 @@
     av_push(av,sv_bless(rv,hv));
     SvREFCNT_dec(sv);
     info=GetPackageInfo(aTHX_ (void *) av,info);
-    (void) strncpy(image->filename,info->image_info->filename,MaxTextExtent-1);
+    (void) MagickStrlCpy(image->filename,info->image_info->filename,sizeof(image->filename));
     SetImageInfo(info->image_info,SETMAGICK_WRITE,&image->exception);
     if (exception.severity != UndefinedException)
       CatchException(&exception);
@@ -6848,8 +6911,8 @@
     GetExceptionInfo(&exception);
     for (i=0; i < n; i++)
     {
-      (void) strncpy(package_info->image_info->filename,list[i],
-        MaxTextExtent-1);
+      (void) MagickStrlCpy(package_info->image_info->filename,list[i],
+                           sizeof(package_info->image_info->filename));
       image=PingImage(package_info->image_info,&exception);
       if (exception.severity != UndefinedException)
         CatchException(&exception);
@@ -7458,7 +7521,7 @@
               PUSHs(&sv_undef);
               continue;
             }
-          (void) strncpy(message,p->name,MaxTextExtent-1);
+          (void) MagickStrlCpy(message,p->name,sizeof(message));
           LocaleLower(message);
           PUSHs(sv_2mortal(newSVpv(message,0)));
         }
@@ -7626,8 +7689,8 @@
     number_images=0;
     for (i=0; i < n; i++)
     {
-      (void) strncpy(package_info->image_info->filename,list[i],
-        MaxTextExtent-1);
+      (void) MagickStrlCpy(package_info->image_info->filename,list[i],
+                           sizeof(package_info->image_info->filename));
       image=ReadImage(package_info->image_info,&exception);
       if (exception.severity != UndefinedException)
         CatchException(&exception);
@@ -8005,11 +8068,11 @@
       if (items > 2)
         for (i=2; i < items; i+=2)
           SetAttribute(aTHX_ package_info,image,SvPV(ST(i-1),na),ST(i));
-    (void) strncpy(filename,package_info->image_info->filename,MaxTextExtent-1);
+    (void) MagickStrlCpy(filename,package_info->image_info->filename,sizeof(filename));
     scene=0;
     for (next=image; next; next=next->next)
     {
-      (void) strncpy(next->filename,filename,MaxTextExtent-1);
+      (void) MagickStrlCpy(next->filename,filename,sizeof(next->filename));
       next->scene=scene++;
     }
     (void) SetImageInfo(package_info->image_info,
diff -r cfb1055afd82 -r 3c58991d3b9b www/Changelog.html
--- a/www/Changelog.html	Fri Dec 24 12:46:36 2021 -0600
+++ b/www/Changelog.html	Fri Dec 24 17:28:54 2021 -0600
@@ -37,6 +37,13 @@
 
 <p>2021-12-24  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>* PerlMagick/Magick.xs: Fix issue that image fill attribute had
+its opacity reset to transparent so it could not be usefully set
+at image scope.  Note that this fix may change results if the fill
+attribute is not set since then the default will actually be used.
+Support Get of &quot;fill&quot; and &quot;stroke&quot;.  Addresses SourceForge issue
+#650 &quot;Image attributes not working and not as documented for
+PerlMagick&quot;.  Eliminate use of dangerous strncpy().</p>
 <p>* magick/transform.c (TransformImage): Trace crop geometry. Trace
 transform geometry.
 (CropImage): Trace crop geometry. Trace bounding page.</p>
diff -r cfb1055afd82 -r 3c58991d3b9b www/INSTALL-unix.html
--- a/www/INSTALL-unix.html	Fri Dec 24 12:46:36 2021 -0600
+++ b/www/INSTALL-unix.html	Fri Dec 24 17:28:54 2021 -0600
@@ -751,12 +751,19 @@
 install GraphicsMagick prior to building PerlMagick in order to
 achieve a working PerlMagick since otherwise the wrong
 GraphicsMagick libraries may be used.</p>
-<p class="last">If the argument ''--with-perl=/path/to/perl'' is supplied, then
+<p>If the argument ''--with-perl=/path/to/perl'' is supplied, then
 /path/to/perl will be taken as the PERL interpreter to use. This is
 important in case the 'perl' executable in your PATH is not PERL5,
 or is not the PERL you want to use.  Experience suggests that static
 PerlMagick builds may not be fully successful (at least for
 executing the test suite) for Perl versions newer than 5.8.8.</p>
+<p class="last">As a convenience, the Makefile targets 'perl-build',
+'install-exec-perl', and 'perl-check' are provided.  In order to
+assure that library dependencies and search paths are correct, it is
+necessary to first install GraphicsMagick via 'make install', then
+build PerlMagick using 'make perl-build', then install PerlMagick
+using 'sudo make install-exec-perl', and then 'make perl-check' to
+make sure that it actually works.</p>
 </td></tr>
 <tr><td class="option-group" colspan="2">
 <kbd><span class="option">--with-perl-options</span></kbd></td>
diff -r cfb1055afd82 -r 3c58991d3b9b www/INSTALL-unix.rst
--- a/www/INSTALL-unix.rst	Fri Dec 24 12:46:36 2021 -0600
+++ b/www/INSTALL-unix.rst	Fri Dec 24 17:28:54 2021 -0600
@@ -703,6 +703,14 @@
   PerlMagick builds may not be fully successful (at least for
   executing the test suite) for Perl versions newer than 5.8.8.
 
+  As a convenience, the Makefile targets 'perl-build',
+  'install-exec-perl', and 'perl-check' are provided.  In order to
+  assure that library dependencies and search paths are correct, it is
+  necessary to first install GraphicsMagick via 'make install', then
+  build PerlMagick using 'make perl-build', then install PerlMagick
+  using 'sudo make install-exec-perl', and then 'make perl-check' to
+  make sure that it actually works.
+
 --with-perl-options
 
   The PerlMagick module is normally installed using the Perl
diff -r cfb1055afd82 -r 3c58991d3b9b www/perl.html
--- a/www/perl.html	Fri Dec 24 12:46:36 2021 -0600
+++ b/www/perl.html	Fri Dec 24 17:28:54 2021 -0600
@@ -43,7 +43,7 @@
 <li><a class="reference internal" href="#introduction" id="id1">Introduction</a></li>
 <li><a class="reference internal" href="#installation" id="id2">Installation</a><ul>
 <li><a class="reference internal" href="#unix" id="id3">UNIX</a></li>
-<li><a class="reference internal" href="#windows-xp-windows-8" id="id4">Windows XP - Windows 8</a></li>
+<li><a class="reference internal" href="#windows-xp-windows-12" id="id4">Windows XP - Windows 12</a></li>
 <li><a class="reference internal" href="#running-the-regression-tests" id="id5">Running the Regression Tests</a></li>
 </ul>
 </li>
@@ -80,11 +80,9 @@
 <p>For Unix, you typically need to be root to install the software. There
 are ways around this. Consult the Perl manual pages for more information.</p>
 </div>
-<div class="section" id="windows-xp-windows-8">
-<h2><a class="toc-backref" href="#id4">Windows XP - Windows 8</a></h2>
-<p>Please note that a nice GUI installer is available for GraphicsMagick.
-PerlMagick is included in this installer. If you are using the installer,
-then there is no need to compile PerlMagick.</p>
+<div class="section" id="windows-xp-windows-12">
+<h2><a class="toc-backref" href="#id4">Windows XP - Windows 12</a></h2>
+<p>[ These procedures have not been exercised for some time ]</p>
 <p>After GraphicsMagick has been compiled from the GraphicsMagick Windows
 source distribution using Microsoft Visual C++, PerlMagick may be
 manually built and installed by opening a CLI window and performing the
diff -r cfb1055afd82 -r 3c58991d3b9b www/perl.rst
--- a/www/perl.rst	Fri Dec 24 12:46:36 2021 -0600
+++ b/www/perl.rst	Fri Dec 24 17:28:54 2021 -0600
@@ -34,12 +34,10 @@
 For Unix, you typically need to be root to install the software. There
 are ways around this. Consult the Perl manual pages for more information.
 
-Windows XP - Windows 8
-----------------------
+Windows XP - Windows 12
+-----------------------
 
-Please note that a nice GUI installer is available for GraphicsMagick.
-PerlMagick is included in this installer. If you are using the installer,
-then there is no need to compile PerlMagick.
+[ These procedures have not been exercised for some time ]
 
 After GraphicsMagick has been compiled from the GraphicsMagick Windows
 source distribution using Microsoft Visual C++, PerlMagick may be
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.