On Thu, 22 Nov 2007, Merijn Broeren wrote:

> 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.

Yeah, that's an old issue that dates back to Log4perl 0.42: There we
figured out that if someone wrote

    $SIG{__DIE__} = sub { print "die handler!\n"; };

and used Log4perl and some module like Time::HiRes is missing, which
is acceptable, the handler gets triggered because eval { require }
causes an exception. Confusion ensued.

Now as it has been pointed out, it's actually the user's fault. A
correct die-Handler checks if it was called because of an eval and
ignores the exception.

Now, I've always thought that this is a bug in perl, nobody writes die
die handlers 'correctly'. To comply with users who get confused with this
confusing behaviour, I put this module_available() hack in place, which,
admittedly, sucks.

What to do? Going back to eval { require } would break every user with
sloppy die handlers.

We could add a load time option to establish the correct behavior:

    use Log::Log4perl qw(:module_check_via_eval);

Other ideas?

-- Mike

Mike Schilli
[EMAIL PROTECTED]

>
> 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
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2005.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> log4perl-devel mailing list
> log4perl-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/log4perl-devel
>

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
log4perl-devel mailing list
log4perl-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/log4perl-devel

Reply via email to