Mailing List Archive: ModPerl: Dev

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

Index | Next | Previous | View Threaded

torsten.foertsch at gmx

Aug 23, 2009, 9:03 AM

Post #1 of 2 (1400 views)
Permalink

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

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

Attachments:

MPMM.patch (2.07 KB)

fred at redhotpenguin

Aug 23, 2009, 1:28 PM

Post #2 of 2 (1300 views)
Permalink

Re: locality of $_ in ModPerl::MM and the like [In reply to]

On Sun, Aug 23, 2009 at 9:03 AM, Torsten
Foertsch<torsten.foertsch [at] gmx> 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 [at] perl
For additional commands, e-mail: dev-help [at] perl

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads