From: Tony Cook <tony@develop-help.com>
Date: Sat, 21 May 2011 04:29:57 +0000 (+1000)
Subject: re-work document and test Imager's logging facility
X-Git-Tag: v0.84~28
X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/10ea52a3d4a66a82193e59839e580cf7b4e27d0b
re-work document and test Imager's logging facility
---
diff --git a/Changes b/Changes
index e1cd1f0e..156672bf 100644
--- 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
===========
diff --git a/Imager.pm b/Imager.pm
index 010b0c9b..d27667a1 100644
--- 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
diff --git a/Imager.xs b/Imager.xs
index f0afbe57..07462bc5 100644
--- 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)
diff --git a/lib/Imager/ImageTypes.pod b/lib/Imager/ImageTypes.pod
index 840a1bb3..6b6ea60a 100644
--- a/lib/Imager/ImageTypes.pod
+++ b/lib/Imager/ImageTypes.pod
@@ -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 a32db1b5..05197707 100644
--- 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 5b1f4839..6cec6eb3 100644
--- a/log.h
+++ b/log.h
@@ -10,10 +10,10 @@
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, ...))
diff --git a/t/t01introvert.t b/t/t01introvert.t
index 77e863f8..be1baf04 100644
--- a/t/t01introvert.t
+++ b/t/t01introvert.t
@@ -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);
diff --git a/t/t93podcover.t b/t/t93podcover.t
index 994f8312..775e0d38 100644
--- a/t/t93podcover.t
+++ b/t/t93podcover.t
@@ -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
index 00000000..ce3323a4
--- /dev/null
+++ b/t/t95log.t
@@ -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");
+}