re-work document and test Imager's logging facility
authorTony Cook <tony@develop-help.com>
Sat, 21 May 2011 04:29:57 +0000 (14:29 +1000)
committerTony Cook <tony@develop-help.com>
Sat, 21 May 2011 04:29:57 +0000 (14:29 +1000)
Changes
Imager.pm
Imager.xs
lib/Imager/ImageTypes.pod
log.c
log.h
t/t01introvert.t
t/t93podcover.t
t/t95log.t [new file with mode: 0644]

diff --git a/Changes b/Changes
index e1cd1f0..156672b 100644 (file)
--- a/Changes
+++ b/Changes
@@ -6,6 +6,11 @@ Imager 0.84
  - Imager no longer inherits from Exporter (unless you're running an
    old, old perl.
 
+Bug fixes:
+
+ - re-work, document and test Imager's logging facility.
+   https://rt.cpan.org/Ticket/Display.html?id=65227
+
 Imager 0.83 - 21 May 2011
 ===========
 
index 010b0c9..d27667a 100644 (file)
--- a/Imager.pm
+++ b/Imager.pm
@@ -453,15 +453,14 @@ sub import {
 }
 
 sub init_log {
-  i_init_log($_[0],$_[1]);
-  i_log_entry("Imager $VERSION starting\n", 1);
+  Imager->open_log(log => $_[0], level => $_[1]);
 }
 
 
 sub init {
   my %parms=(loglevel=>1,@_);
   if ($parms{'log'}) {
-    init_log($parms{'log'},$parms{'loglevel'});
+    Imager->open_log(log => $parms{log}, level => $parms{loglevel});
   }
 
   if (exists $parms{'warn_obsolete'}) {
@@ -475,6 +474,42 @@ sub init {
   }
 }
 
+{
+  my $is_logging = 0;
+
+  sub open_log {
+    my $class = shift;
+    my (%opts) = ( loglevel => 1, @_ );
+
+    $is_logging = i_init_log($opts{log}, $opts{loglevel});
+    unless ($is_logging) {
+      Imager->_set_error(Imager->_error_as_msg());
+      return;
+    }
+
+    Imager->log("Imager $VERSION starting\n", 1);
+
+    return $is_logging;
+  }
+
+  sub close_log {
+    i_init_log(undef, -1);
+    $is_logging = 0;
+  }
+
+  sub log {
+    my ($class, $message, $level) = @_;
+
+    defined $level or $level = 1;
+
+    i_log_entry($message, $level);
+  }
+
+  sub is_logging {
+    return $is_logging;
+  }
+}
+
 END {
   if ($DEBUG) {
     print "shutdown code\n";
@@ -4255,6 +4290,9 @@ box() - L<Imager::Draw/box()> - draw a filled or outline box.
 
 circle() - L<Imager::Draw/circle()> - draw a filled circle
 
+close_log() - L<Imager::ImageTypes/close_log()> - close the Imager
+debugging log.
+
 colorcount() - L<Imager::ImageTypes/colorcount()> - the number of
 colors in an image's palette (paletted images only)
 
@@ -4338,10 +4376,16 @@ is_bilevel() - L<Imager::ImageTypes/is_bilevel()> - returns whether
 image write functions should write the image in their bilevel (blank
 and white, no gray levels) format
 
+is_logging() L<Imager::ImageTypes/is_logging()> - test if the debug
+log is active.
+
 line() - L<Imager::Draw/line()> - draw an interval
 
 load_plugin() - L<Imager::Filters/load_plugin()>
 
+log() - L<Imager::ImageTypes/log()> - send a message to the debugging
+log.
+
 map() - L<Imager::Transformations/"Color Mappings"> - remap color
 channel values
 
@@ -4367,6 +4411,8 @@ NF() - L<Imager::Handy/NF()>
 
 open() - L<Imager::Files> - an alias for read()
 
+open_log() - L<Imager::ImageTypes/open_log()> - open the debug log.
+
 =for stopwords IPTC
 
 parseiptc() - L<Imager::Files/parseiptc()> - parse IPTC data from a JPEG
index f0afbe5..07462bc 100644 (file)
--- a/Imager.xs
+++ b/Imager.xs
@@ -862,6 +862,12 @@ validate_i_ppal(i_img *im, i_palidx const *indexes, int count) {
 #define ICLF_new_internal(r, g, b, a) i_fcolor_new((r), (g), (b), (a))
 #define ICLF_DESTROY(cl) i_fcolor_destroy(cl)
 
+#ifdef IMAGER_LOG
+#define i_log_enabled() 1
+#else
+#define i_log_enabled() 0
+#endif
+
 #if i_int_hlines_testing()
 
 typedef i_int_hlines *Imager__Internal__Hlines;
@@ -1320,20 +1326,24 @@ i_sametype_chans(im, x, y, channels)
                int y
                int channels
 
-void
+int
 i_init_log(name_sv,level)
              SV*    name_sv
               int     level
        PREINIT:
          const char *name = SvOK(name_sv) ? SvPV_nolen(name_sv) : NULL;
        CODE:
-         i_init_log(name, level);
+         RETVAL = i_init_log(name, level);
+       OUTPUT:
+         RETVAL
 
 void
 i_log_entry(string,level)
              char*    string
               int     level
 
+int
+i_log_enabled()
 
 void
 i_img_exorcise(im)
index 840a1bb..6b6ea60 100644 (file)
@@ -1120,9 +1120,10 @@ This function is a mess, it can take the following named parameters:
 
 =item *
 
-C<log> - name of a log file to log Imager's actions to.  Not all actions
-are logged, but the debugging memory allocator does log allocations
-here.  Ignored if Imager has been built without logging support.
+C<log> - name of a log file to log Imager's actions to.  Not all
+actions are logged, but the debugging memory allocator does log
+allocations here.  Ignored if Imager has been built without logging
+support.  Preferably use the open_log() method instead.
 
 =item *
 
@@ -1147,6 +1148,69 @@ Example:
 
 =back
 
+=head1 LOGGING METHODS
+
+Imager can open an internal log to send debugging information to.
+This log is extensively used in Imager's tests, but you're unlikely to
+use it otherwise.
+
+If Imager has been built with logging disabled, the methods fail
+quietly.
+
+=over
+
+=item open_log()
+
+Open the Imager debugging log file.
+
+=over
+
+=item *
+
+C<log> - the file name to log to.  If this is undef logging information
+is sent to the standard error stream.
+
+=item *
+
+C<loglevel> the level of logging to produce.  Default: 1.
+
+=back
+
+Returns a true value if the log file was opened successfully.
+
+  # send debug output to test.log
+  Imager->open_log(log => "test.log");
+
+  # send debug output to stderr
+  Imager->open_log();
+
+=item close_log()
+
+Close the Imager debugging log file and disable debug logging.
+
+No parameters.
+
+  Imager->close_log();
+
+=item log()
+
+ Imager->log($message)
+ Imager->log($message, $level)
+
+This method does not use named parameters.
+
+The default for C<$level> is 1.
+
+Send a message to the debug log.
+
+  Imager->log("My code got here!");
+
+=item is_logging()
+
+Returns a true value if logging is enabled.
+
+=back
+
 =head1 REVISION
 
 $Revision$
diff --git a/log.c b/log.c
index a32db1b..0519770 100644 (file)
--- a/log.c
+++ b/log.c
@@ -1,6 +1,9 @@
 #include "imconfig.h"
 #include "log.h"
 #include <stdlib.h>
+#include <errno.h>
+
+#ifdef IMAGER_LOG
 
 #define DTBUFF 50
 #define DATABUFF DTBUFF+3+10+1+5+1+1
@@ -12,14 +15,13 @@ static char  date_buffer[DTBUFF];
 static char  data_buffer[DATABUFF];
 
 
-#ifdef IMAGER_LOG
-
 /*
  * Logging is active
  */
 
-void
+int
 i_init_log(const char* name,int level) {
+  i_clear_error();
   log_level = level;
   if (level < 0) {
     lg_file = NULL;
@@ -28,13 +30,17 @@ i_init_log(const char* name,int level) {
       lg_file = stderr;
     } else {
       if (NULL == (lg_file = fopen(name, "w+")) ) { 
-       fprintf(stderr,"Cannot open file '%s'\n",name);
-       exit(2);
+       i_push_errorf(errno, "Cannot open file '%s': (%d)", name, errno);
+       return 0;
       }
     }
   }
-  setvbuf(lg_file, NULL, _IONBF, BUFSIZ);
-  mm_log((0,"Imager - log started (level = %d)\n", level));
+  if (lg_file) {
+    setvbuf(lg_file, NULL, _IONBF, BUFSIZ);
+    mm_log((0,"Imager - log started (level = %d)\n", level));
+  }
+
+  return lg_file != NULL;
 }
 
 void
@@ -55,17 +61,6 @@ i_fatal(int exitcode,const char *fmt, ... ) {
   exit(exitcode);
 }
 
-#else
-
-/*
- * Logging is inactive - insert dummy functions
- */
-
-void i_init_log(const char* name,int onoff) {}
-void i_fatal(int exitcode,const char *fmt, ... ) { exit(exitcode); }
-
-
-#endif
 
 /*
 =item i_loog(level, format, ...)
@@ -111,3 +106,26 @@ i_lhead(const char *file, int line) {
     sprintf(data_buffer, "[%s] %10s:%-5d ", date_buffer, file, line);
   }
 }
+
+#else
+
+/*
+ * Logging is inactive - insert dummy functions
+ */
+
+int i_init_log(const char* name,int onoff) {
+  i_clear_error();
+  i_push_error(0, "Logging disabled");
+  return 0;
+}
+
+void i_fatal(int exitcode,const char *fmt, ... ) { exit(exitcode); }
+
+void
+i_loog(int level,const char *fmt, ... ) {
+}
+
+void
+i_lhead(const char *file, int line) { }
+
+#endif
diff --git a/log.h b/log.h
index 5b1f483..6cec6eb 100644 (file)
--- a/log.h
+++ b/log.h
    global: creates a global variable FILE* lg_file
 */
 
+int i_init_log( const char *name, int onoff );
+void i_fatal ( int exitcode,const char *fmt, ... );
 void i_lhead ( const char *file, int line  );
-void i_init_log( const char *name, int onoff );
 void i_loog(int level,const char *msg, ... );
-void i_fatal ( int exitcode,const char *fmt, ... );
 
 /*
 =item mm_log((level, format, ...))
index 77e863f..be1baf0 100644 (file)
@@ -11,7 +11,7 @@ use Imager::Test qw(image_bounds_checks is_color3 is_color4 is_fcolor4 color_cmp
 
 -d "testout" or mkdir "testout";
 
-init_log("testout/t01introvert.log",1);
+Imager->open_log(log => "testout/t01introvert.log");
 
 my $im_g = Imager::ImgRaw::new(100, 101, 1);
 
@@ -542,6 +542,12 @@ cmp_ok(Imager->errstr, '=~', qr/channels must be between 1 and 4/,
   image_bounds_checks($im);
 }
 
+Imager->close_log();
+
+unless ($ENV{IMAGER_KEEP_FILES}) {
+  unlink "testout/t01introvert.log";
+}
+
 sub check_add {
   my ($im, $color, $expected) = @_;
   my $index = Imager::i_addcolors($im, $color);
index 994f831..775e0d3 100644 (file)
@@ -17,7 +17,6 @@ my @private =
    '^DSO_',
    '^Inline$',
    '^yatf$',
-   '^m_init_log$',
    '^malloc_state$',
    '^init_log$',
    '^polybezier$', # not ready for public consumption
diff --git a/t/t95log.t b/t/t95log.t
new file mode 100644 (file)
index 0000000..ce3323a
--- /dev/null
@@ -0,0 +1,34 @@
+#!perl -w
+use strict;
+use Imager;
+use Test::More tests => 6;
+
+my $log_name = "testout/t95log.log";
+
+my $log_message = "test message 12345";
+
+SKIP: {
+  skip("Logging not build", 3)
+    unless Imager::i_log_enabled();
+  ok(Imager->open_log(log => $log_name), "open log")
+    or diag("Open log: " . Imager->errstr);
+  ok(-f $log_name, "file is there");
+  Imager->log($log_message);
+  Imager->close_log();
+
+  my $data = '';
+  if (open LOG, "< $log_name") {
+    $data = do { local $/; <LOG> };
+    close LOG;
+  }
+  like($data, qr/\Q$log_message/, "check message made it to the log");
+}
+
+SKIP: {
+  skip("Logging built", 3)
+    if Imager::i_log_enabled();
+
+  ok(!Imager->open_log(log => $log_name), "should be no logfile");
+  is(Imager->errstr, "Logging disabled", "check error message");
+  ok(!-f $log_name, "file shouldn't be there");
+}