locality of $_ in ModPerl::MM and the like

[Torsten Foertsch <to...@gmx.net>]

Hi,

recently I stumbled upon a curious thing. A freshly unpacked modperl 
aborted compilation after compiling in src/modules/perl. These are the 
symptoms:

- during perl Makefile.PL:

...
WARNING: #define AP_SERVER_MAJORVERSION_NUMBER 2
 is not a known parameter.
Checking if your kit is complete...
Looks good
Writing Makefile for Apache2::Reload
!!! no default argument defined for argument: #define 
AP_SERVER_MAJORVERSION_NUMBER 2
 at ./Makefile.PL line 60
WARNING: #define AP_SERVER_MAJORVERSION_NUMBER 2
 is not a known parameter.
Checking if your kit is complete...
Looks good

- and then in make:

...
cp lib/Apache2/porting.pm blib/lib/Apache2/porting.pm
make[1]: Entering directory 
`/usr/src/packages/BUILD/mod_perl-2.0.5threading3/Apache-Reload'
Makefile:14: *** empty variable name.  Stop.
make[1]: Leaving directory 
`/usr/src/packages/BUILD/mod_perl-2.0.5threading3/Apache-Reload'
make: *** [subdirs] Error 2

A second call of "perl Makefile.PL; make" made it work normally.

I could track it down to $_ being used as loop variable and overwritten 
by a function called in that loop in ModPerl::MM.

There are obviously 2 ways to fix that. First, introduce a local loop 
variable and second, ensure $_ is not changed inside a function unless 
this is the purpose of the function.

What is the way to go? Or are both ways ok?

This patch introduces $o as loop variable instead of $_. For me the
@default_opts loop was the culprit but I have changed all loops in
WriteMakefile().

Index: lib/ModPerl/MM.pm
===================================================================
--- lib/ModPerl/MM.pm	(revision 806134)
+++ lib/ModPerl/MM.pm	(working copy)
@@ -132,22 +132,22 @@
     my_import(__PACKAGE__);
 
     # set top-level WriteMakefile's values if weren't set already
-    for (@default_opts) {
-        $args{$_} = get_def_opt($_) unless exists $args{$_}; # already defined
+    for my $o (@default_opts) {
+        $args{$o} = get_def_opt($o) unless exists $args{$o}; # already defined
     }
 
     # set dynamic_lib-level WriteMakefile's values if weren't set already
     $args{dynamic_lib} ||= {};
     my $dlib = $args{dynamic_lib};
-    for (@default_dlib_opts) {
-        $dlib->{$_} = get_def_opt($_) unless exists $dlib->{$_};
+    for my $o (@default_dlib_opts) {
+        $dlib->{$o} = get_def_opt($o) unless exists $dlib->{$o};
     }
 
     # set macro-level WriteMakefile's values if weren't set already
     $args{macro} ||= {};
     my $macro = $args{macro};
-    for (@default_macro_opts) {
-        $macro->{$_} = get_def_opt($_) unless exists $macro->{$_};
+    for my $o (@default_macro_opts) {
+        $macro->{$o} = get_def_opt($o) unless exists $macro->{$o};
     }
 
     ExtUtils::MakeMaker::WriteMakefile(%args);

And this one prevents inc() from changing $_:

Index: lib/Apache2/Build.pm
===================================================================
--- lib/Apache2/Build.pm	(revision 806134)
+++ lib/Apache2/Build.pm	(working copy)
@@ -2068,6 +2068,7 @@
 }
 
 sub inc {
+    local $_;
     my @includes = map { "-I$_" } @{ shift->includes };
     "@includes";
 }


The attached patch contains these 2 plus a fix for the typo mentioned
in my previous mail.

Can I apply it to trunk?

Torsten

Re: locality of $_ in ModPerl::MM and the like

[Fred Moyer <fr...@redhotpenguin.com>]

On Sun, Aug 23, 2009 at 9:03 AM, Torsten
Foertsch<to...@gmx.net> wrote:
> Hi,
>
> recently I stumbled upon a curious thing. A freshly unpacked modperl
> aborted compilation after compiling in src/modules/perl. These are the
> symptoms:

....


+1 to this patch.  Has the added benefit of using a lexical loop
variable which is a better practice than 'for ... $_' in my humble
opinion.

> This patch introduces $o as loop variable instead of $_. For me the
> @default_opts loop was the culprit but I have changed all loops in
> WriteMakefile().
>
> Index: lib/ModPerl/MM.pm
> ===================================================================
> --- lib/ModPerl/MM.pm   (revision 806134)
> +++ lib/ModPerl/MM.pm   (working copy)
> @@ -132,22 +132,22 @@
>     my_import(__PACKAGE__);
>
>     # set top-level WriteMakefile's values if weren't set already
> -    for (@default_opts) {
> -        $args{$_} = get_def_opt($_) unless exists $args{$_}; # already defined
> +    for my $o (@default_opts) {
> +        $args{$o} = get_def_opt($o) unless exists $args{$o}; # already defined
>     }
>
>     # set dynamic_lib-level WriteMakefile's values if weren't set already
>     $args{dynamic_lib} ||= {};
>     my $dlib = $args{dynamic_lib};
> -    for (@default_dlib_opts) {
> -        $dlib->{$_} = get_def_opt($_) unless exists $dlib->{$_};
> +    for my $o (@default_dlib_opts) {
> +        $dlib->{$o} = get_def_opt($o) unless exists $dlib->{$o};
>     }
>
>     # set macro-level WriteMakefile's values if weren't set already
>     $args{macro} ||= {};
>     my $macro = $args{macro};
> -    for (@default_macro_opts) {
> -        $macro->{$_} = get_def_opt($_) unless exists $macro->{$_};
> +    for my $o (@default_macro_opts) {
> +        $macro->{$o} = get_def_opt($o) unless exists $macro->{$o};
>     }
>
>     ExtUtils::MakeMaker::WriteMakefile(%args);

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org