From: Tony Cook <tony@develop=help.com>
Date: Mon, 18 Oct 2004 23:52:10 +0000 (+0000)
Subject: - the XS code for i_tt_glyph_name() used unsigned char to store a
X-Git-Tag: Imager-0.48^2~307
X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/178922554e30dbbe9a4362f831b800464b26ddd1

- the XS code for i_tt_glyph_name() used unsigned char to store a
  unicode character when it should have used unsigned long.
- the XS code for i_t1_glyph_name() used unsigned char to store a
  unicode character when it should have used unsigned long.
- resolves https://rt.cpan.org/Ticket/Display.html?id=7949
- the type 1 glyph_names() method didn't do correct error handling
  when the string parameter wasn't supplied
---

diff --git a/Changes b/Changes
index 967dbd13..d9c417cf 100644
--- a/Changes
+++ b/Changes
@@ -858,7 +858,6 @@ Revision history for Perl extension Imager.
   names
 - added reliable_only optional parameter to the glyph_names()
   method so you can ignore theresult of FT_Has_PS_Glyph_Names()
-- partly resolves https://rt.cpan.org/Ticket/Display.html?id=7949
 - handle errors given by i_ft2_glyph_name() a bit more
   correctly
 - the FT1 glyph_names() method didn't do correct error handling 
@@ -867,6 +866,13 @@ Revision history for Perl extension Imager.
   wasn't being released (detected with valgrind)
 - the testtools.pl match[nx]() functions escapes the test string on 
   test failure a bit better
+- the XS code for i_tt_glyph_name() used unsigned char to store a 
+  unicode character when it should have used unsigned long.
+- the XS code for i_t1_glyph_name() used unsigned char to store a 
+  unicode character when it should have used unsigned long.
+- resolves https://rt.cpan.org/Ticket/Display.html?id=7949
+- the type 1 glyph_names() method didn't do correct error handling 
+  when the string parameter wasn't supplied
 
 =================================================================
 
diff --git a/Imager.xs b/Imager.xs
index 2e5baba4..aeae14b9 100644
--- a/Imager.xs
+++ b/Imager.xs
@@ -1864,7 +1864,7 @@ i_t1_glyph_name(handle, text_sv, utf8 = 0)
         text = SvPV(text_sv, work_len);
         len = work_len;
         while (len) {
-          unsigned char ch;
+          unsigned long ch;
           if (utf8) {
             ch = i_utf8_advance(&text, &len);
             if (ch == ~0UL) {
@@ -2053,7 +2053,7 @@ i_tt_glyph_name(handle, text_sv, utf8 = 0)
         text = SvPV(text_sv, work_len);
         len = work_len;
         while (len) {
-          unsigned char ch;
+          unsigned long ch;
           if (utf8) {
             ch = i_utf8_advance(&text, &len);
             if (ch == ~0UL) {
diff --git a/lib/Imager/Font/Type1.pm b/lib/Imager/Font/Type1.pm
index bae89699..5c71e621 100644
--- a/lib/Imager/Font/Type1.pm
+++ b/lib/Imager/Font/Type1.pm
@@ -127,7 +127,7 @@ sub glyph_names {
 
   my $string = $input{string};
   defined $string
-    or return Imager->_seterror("no string parameter passed to glyph_names");
+    or return Imager->_set_error("no string parameter passed to glyph_names");
   my $utf8 = _first($input{utf8} || 0);
 
   Imager::i_t1_glyph_name($self->{id}, $string, $utf8);
diff --git a/t/t30t1font.t b/t/t30t1font.t
index 22b12642..810aacd3 100644
--- a/t/t30t1font.t
+++ b/t/t30t1font.t
@@ -8,7 +8,7 @@
 # (It may become useful if the test is moved to ./t subdirectory.)
 use strict;
 my $loaded;
-BEGIN { $| = 1; print "1..38\n"; }
+BEGIN { $| = 1; print "1..41\n"; }
 END {print "not ok 1\n" unless $loaded;}
 use Imager qw(:all);
 use Imager::Color;
@@ -28,13 +28,13 @@ my $fontname_afm=$ENV{'T1FONTTESTAFM'}||'./fontfiles/dcr10.afm';
 
 
 if (!(i_has_format("t1")) ) {
-  skipx(37, "t1lib unavailable or disabled");
+  skipx(40, "t1lib unavailable or disabled");
 }
 elsif (! -f $fontname_pfb) {
-  skipx(37, "cannot find fontfile for type 1 test $fontname_pfb");
+  skipx(40, "cannot find fontfile for type 1 test $fontname_pfb");
 }
 elsif (! -f $fontname_afm) {
-  skipx(37, "cannot find fontfile for type 1 test $fontname_afm");
+  skipx(40, "cannot find fontfile for type 1 test $fontname_afm");
 } else {
 
   print "# has t1\n";
@@ -189,12 +189,25 @@ elsif (! -f $fontname_afm) {
     okx($face_name eq 'ExistenceTest', "face name");
 
     my @glyph_names = $font->glyph_names(string=>"!J/");
-    okx($glyph_names[0] eq 'exclam', "check exclam name OO");
+    isx($glyph_names[0], 'exclam', "check exclam name OO");
     okx(!defined($glyph_names[1]), "check for no J name OO");
-    okx($glyph_names[2] eq 'slash', "check slash name OO");
+    isx($glyph_names[2], 'slash', "check slash name OO");
+
+    # this character chosen since when it's truncated to one byte it
+    # becomes 0x21 or '!' which the font does define
+    my $text = pack("C*", 0xE2, 0x80, 0xA1); # "\x{2021}" as utf-8
+    @glyph_names = $font->glyph_names(string=>$text, utf8=>1);
+    isx($glyph_names[0], undef, "expect no glyph_name for \\x{20A1}");
+
+    # make sure a missing string parameter is handled correctly
+    eval {
+      $font->glyph_names();
+    };
+    isx($@, "", "correct error handling");
+    matchx(Imager->errstr, qr/no string parameter/, "error message");
   }
   else {
-    skipx(12, "Could not load test font");
+    skipx(15, "Could not load test font");
   }
 }
 
diff --git a/t/t35ttfont.t b/t/t35ttfont.t
index c2dc3dfd..5e405576 100644
--- a/t/t35ttfont.t
+++ b/t/t35ttfont.t
@@ -7,7 +7,7 @@
 # (It may become useful if the test is moved to ./t subdirectory.)
 use strict;
 my $loaded;
-BEGIN { $| = 1; print "1..35\n"; }
+BEGIN { $| = 1; print "1..39\n"; }
 END {print "not ok 1\n" unless $loaded;}
 use Imager qw(:all);
 require "t/testtools.pl";
@@ -18,7 +18,7 @@ okx(1, "Loaded");
 init_log("testout/t35ttfont.log",2);
 
 unless (i_has_format("tt")) { 
-  skipx(34, "freetype 1.x unavailable or disabled");
+  skipx(38, "freetype 1.x unavailable or disabled");
   malloc_state();
   exit;
 }
@@ -29,7 +29,7 @@ my $fontname=$ENV{'TTFONTTEST'} || $deffont;
 
 if (! -f $fontname) {
   print "# cannot find fontfile for truetype test $fontname\n";
-  skipx(34, 'Cannot load test font');	
+  skipx(38, 'Cannot load test font');	
   exit;
 }
 
@@ -163,4 +163,24 @@ else {
 }
 undef $hcfont;
 
+my $name_font = "fontfiles/NameTest.ttf";
+$hcfont = Imager::Font->new(file=>$name_font);
+if (okx($hcfont, "loading name font")) {
+  # make sure a missing string parameter is handled correctly
+  eval {
+    $hcfont->glyph_names();
+  };
+  isx($@, "", "correct error handling");
+  matchx(Imager->errstr, qr/no string parameter/, "error message");
+
+  my $text = pack("C*", 0xE2, 0x80, 0x90); # "\x{2010}" as utf-8
+  my @names = $hcfont->glyph_names(string=>$text, utf8=>1);
+  isx($names[0], "hyphentwo", "check utf8 glyph name");
+}
+else {
+  skipx(3, "could not load name font $name_font");
+}
+
+undef $hcfont;
+
 okx(1, "end of code");
diff --git a/t/t38ft2font.t b/t/t38ft2font.t
index 067d5d9e..5f33a25c 100644
--- a/t/t38ft2font.t
+++ b/t/t38ft2font.t
@@ -18,7 +18,7 @@ okx(1, "loaded");
 init_log("testout/t38ft2font.log",2);
 
 if (!(i_has_format("ft2")) ) { 
-  skipx(115, "No freetype2 library found");
+  skipx(119, "No freetype2 library found");
   exit;
 }
 print "# has ft2\n";
@@ -26,7 +26,7 @@ print "# has ft2\n";
 $fontname=$ENV{'TTFONTTEST'}||'./fontfiles/dodge.ttf';
 
 if (! -f $fontname) {
-  skipx(124, "cannot find fontfile $fontname");
+  skipx(119, "cannot find fontfile $fontname");
   malloc_state();
   exit;
 }
diff --git a/t/testtools.pl b/t/testtools.pl
index d5b2a242..ad4886f9 100644
--- a/t/testtools.pl
+++ b/t/testtools.pl
@@ -43,13 +43,13 @@ sub skipx {
   $TESTNUM += $count;
 }
 
-sub okx {
+sub okx ($$) {
   my ($ok, $comment) = @_;
 
   return okn($TESTNUM++, $ok, $comment);
 }
 
-sub okn {
+sub okn ($$$) {
   my ($num, $ok, $comment) = @_;
 
   defined $num or confess "No \$num supplied";