[log4perl-devel] Log::Log4Perl::Util::module_available broken...

[Merijn B. <me...@il...>]

Hello,

There is an existing RT bug for the issue of module_available being
broken already:

http://rt.cpan.org/Public/Bug/Display.html?id=24884

which is 10 months older then the release done yesterday. Since there
have been tickets closed since then I am assuming you guys are aware of
this issue already. 

I just wanted to reiterate that this code really is broken, both in
spirit and in implementation. Please reconsider having this broken code
in such a useful package. The patch in RT is sufficient to restore
functionality, but I would personally take out all module_available
calls everywhere from Log4perl and replace it with a simple eval {require }. 

To illustrate implementation brokenness, see line 33 of Util.pm:

	ref $dir !~ /^(GLOB|SCALAR|HASH|REF|LVALUE)$/) {

which tries to determine the reference status on the result of a regular
expression. I assume the author wanted to write this :

	ref($dir) !~ /^(GLOB|SCALAR|HASH|REF|LVALUE)$/) {

but forgot about operator precedence. So this test will *never* be true. 
Which is good, as the code on line 34 inside the if branch is bad as well. 

	return 1 if $dir->INC();

as the INC method is supposed to return the open file handle of the file
requested. Since you do not request a file, the method is under no
obligation to actually return a true value.

But it would be pointless to try fixing this implementation since
whatever the result of the module_available function is, it just leads
to an eval { require } afterwards anyway, so you are doubling the
effort. It would be very nice if you would consider to modify Log4perl
to not use module_available. Or accept the RT patch. The current code
does not work with @INC objects. 

Regards,
-- 
Merijn Broeren | We take risks, we know we take them. Therefore, when things
               | come out against us, we have no cause for complaint.
               | - Scott, last journal entry, march 1912