From: Tony Cook <tony@develop-help.com>
Date: Fri, 12 Oct 2012 00:35:45 +0000 (+1100)
Subject: re-work GIF version setting to support the upcoming 5.0.1 mechanism
X-Git-Tag: v0.93~10
X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/a93c43b4441dfc08c0abd1b9565236cd29aded9f

re-work GIF version setting to support the upcoming 5.0.1 mechanism

also test that it works, since it won't for 4.2.0 and 5.0.0
---

diff --git a/GIF/MANIFEST b/GIF/MANIFEST
index 4f7bd9f6..38264c98 100644
--- a/GIF/MANIFEST
+++ b/GIF/MANIFEST
@@ -11,6 +11,8 @@ README
 t/t10gif.t
 t/t20new.t
 t/t30fixed.t
+t/t40limit.t
+t/t50header.t
 testimg/badindex.gif
 testimg/bandw.gif
 testimg/expected.gif
diff --git a/GIF/imgif.c b/GIF/imgif.c
index c8e2262e..a0c31b72 100644
--- a/GIF/imgif.c
+++ b/GIF/imgif.c
@@ -60,6 +60,7 @@ functionality with giflib3.
 */
 
 #if defined(GIFLIB_MAJOR) && GIFLIB_MAJOR >= 5
+#define POST_SET_VERSION
 #define myDGifOpen(userPtr, readFunc, Error) DGifOpen((userPtr), (readFunc), (Error))
 #define myEGifOpen(userPtr, readFunc, Error) EGifOpen((userPtr), (readFunc), (Error))
 #define myGifError(gif) ((gif)->Error)
@@ -67,6 +68,7 @@ functionality with giflib3.
 #define FreeMapObject GifFreeMapObject
 
 #else
+#define PRE_SET_VERSION
 static GifFileType *
 myDGifOpen(void *userPtr, InputFunc readFunc, int *error) {
   GifFileType *result = DGifOpen(userPtr, readFunc);
@@ -1212,43 +1214,16 @@ make_gif_map(i_quantize *quant, i_img *img, int want_trans) {
 }
 
 /*
-=item gif_set_version(i_quantize *quant, i_img *imgs, int count)
+=item need_version_89a(i_quantize *quant, i_img *imgs, int count)
 
-We need to call EGifSetGifVersion() before opening the file - put that
-common code here.
-
-Unfortunately giflib 4.1.0 crashes when we use this.  Internally
-giflib 4.1.0 has code:
-
-  static char *GifVersionPrefix = GIF87_STAMP;
-
-and the code that sets the version internally does:
-
-  strncpy(&GifVersionPrefix[3], Version, 3);
-
-which is very broken.
-
-Failing to set the correct GIF version doesn't seem to cause a problem
-with readers.
-
-Modern versions (4.1.4 anyway) of giflib/libungif handle
-EGifSetGifVersion correctly.
-
-If t/t105gif.t crashes here then run Makefile.PL with
---nogifsetversion, eg.:
-
-  perl Makefile.PL --nogifsetversion
-
-or install a less buggy giflib.
-
-This code is completely unnecessary in giflib 5
+Return true if the file we're creating on these images needs a GIF89a
+header.
 
 =cut
 */
 
-static void
-gif_set_version(i_quantize *quant, i_img **imgs, int count) {
-#if !defined(GIFLIB_MAJOR) || GIFLIB_MAJOR < 5
+static int
+need_version_89a(i_quantize *quant, i_img **imgs, int count) {
   int need_89a = 0;
   int temp;
   int i;
@@ -1276,11 +1251,8 @@ gif_set_version(i_quantize *quant, i_img **imgs, int count) {
       break;
     }
   }
-  if (need_89a)
-     EGifSetGifVersion("89a");
-  else
-     EGifSetGifVersion("87a");
-#endif
+
+  return need_89a;
 }
 
 static int 
@@ -1857,7 +1829,9 @@ i_writegif_wiol(io_glue *ig, i_quantize *quant, i_img **imgs,
 
   i_clear_error();
 
-  gif_set_version(quant, imgs, count);
+#ifdef PRE_SET_VERSION
+  EGifSetGifVersion(need_version_89a(quant, imgs, count) ? "89a" : "87a");
+#endif
   
   if ((GifFile = myEGifOpen((void *)ig, io_glue_write_cb, &gif_error )) == NULL) {
     gif_push_error(gif_error);
@@ -1866,6 +1840,10 @@ i_writegif_wiol(io_glue *ig, i_quantize *quant, i_img **imgs,
     return 0;
   }
   
+#ifdef POST_SET_VERSION
+  EGifSetGifVersion(GifFile, need_version_89a(quant, imgs, count));
+#endif
+
   result = i_writegif_low(quant, GifFile, imgs, count);
   
   if (i_io_close(ig))
diff --git a/GIF/t/t50header.t b/GIF/t/t50header.t
new file mode 100644
index 00000000..ff00fb71
--- /dev/null
+++ b/GIF/t/t50header.t
@@ -0,0 +1,25 @@
+#!perl -w
+use strict;
+use Test::More tests => 4;
+use Imager;
+use Imager::Test qw(test_image);
+
+# giflib 4.2.0 and 5.0.0 had a bug with producing the wrong
+# GIF87a/GIF89a header, test we get the right header
+# https://rt.cpan.org/Ticket/Display.html?id=79679
+# https://sourceforge.net/tracker/?func=detail&aid=3574283&group_id=102202&atid=631304
+my $im = test_image()->to_paletted();
+
+{
+  my $data;
+  ok($im->write(data => \$data, type => "gif"),
+     "write with no tags, should be GIF87a");
+  is(substr($data, 0, 6), "GIF87a", "check header is GIF87a");
+}
+
+{
+  my $data;
+  ok($im->write(data => \$data, type => "gif", gif_loop => 1),
+     "write with loop tags, should be GIF89a");
+  is(substr($data, 0, 6), "GIF89a", "check header is GIF89a");
+}
diff --git a/MANIFEST b/MANIFEST
index 67abad36..f9703302 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -80,6 +80,7 @@ GIF/t/t10gif.t
 GIF/t/t20new.t
 GIF/t/t30fixed.t
 GIF/t/t40limit.t
+GIF/t/t50header.t
 GIF/testimg/badindex.gif	GIF with a bad color index
 GIF/testimg/bandw.gif
 GIF/testimg/expected.gif