You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Sander Striker <st...@apache.org> on 2001/08/24 23:09:13 UTC

RE: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

> On Fri, 2001-08-24 at 13:50, Sander Striker wrote:
> > Please hlod off for a day.  I've done some pools coding
> > too again (and hopefully I can send in a patch tonight).
> > It should have even better performance.  Just ironing it
> > out.
> > 
> > Thanks,
> > 
> > Sander
> > 
> sander,
> could your and brian's patches be merged, or do they both basiclaly
> do the same thing.

Mine takes it a bit further.  It essentially makes the freelist more
efficient.  Adds the possibility to request a new freelist for a pool,
which can optionally have locking.  Furthermore it resolves some
issues with possible segfaults in the current pools code when running
out of memory.

However, there is a downside.  I started with an empty slate and then
started to add things in.  I still haven't merged some things over
(the APR_POOL_DEBUG mode, USE_MALLOC and other fancy debug stuff).  Also,
my code isn't documented very well at the moment (OTOH, the current
pools code isn't exactly a quick read with all those #if[def] lines...).

Sander


> > 
> > > -----Original Message-----
> > > From: Ian Holsman [mailto:ianh@cnet.com]
> > > Sent: 24 August 2001 21:14
> > > To: dev@httpd.apache.org
> > > Subject: [Fwd: brianp patch Quantify results] (was 
> thread-specific free
> > > listfor pools" patch )
> > > 
> > > 
> > > One of our other developers ran Brian Pane's 
> > > thread-specific free list for pools patch (posted ~1 week ago)
> > > 
> > > 
> > > 
> > > here are his results.
> > > ...Ian
> > > 
> > > -----Forwarded Message-----
> > > From: Blaise Tarr <XXXXXXXXXX>
> > > To: pxi-webserver@cnet.com
> > > Subject: brianp patch Quantify results
> > > 
> > > 
> > > Hey,
> > > 
> > > For the baseline I used the CVS version from yesterday (8/3) morning.
> > > Then I applied Brian's "thread-specific free list for pools" patch.
> > > 
> > > I used these configs:
> > >     StartServers         1 
> > >     MaxClients           1 
> > >     MinSpareThreads      5 
> > >     MaxSpareThreads     10 
> > >     ThreadsPerChild     25 
> > > 
> > > For the test, I requested 500 news.com pages that have 2 virtual
> > > includes.  The pages were copies of the same file but had different
> > > names.  (lynx -source http://hungry.cnet.com/2file/00${i}.html)
> > > 
> > > handle_include + descendants were 9.5% faster with Brian's patch, and
> > > accounted for 5.89% of the total time, as opposed to 6.28% of the
> > > total time for the baseline.  Overall, Brian's patch reduced the
> > > number of cycles by 3.74%.
> > > 
> > > Now, I must add that these are Quantify numbers, not real world
> > > numbers. 
> > > So, what's next?
> > > 
> > > -- 
> > > Blaise Tarr
> > > XXXXXXXXXXXXXXX         CNET.com
> > > 908.541.3771            The source for computers and technology.
> > > 
> > > 
> > > 
> > > 
> -- 
> Ian Holsman          IanH@cnet.com
> Performance Measurement & Analysis
> CNET Networks   -   (415) 364-8608
> 
> 
> 
> 

Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> For the pools code, it can only be patched. It is unacceptable to toss a
> completely written-from-scratch replacement into the code base. If it takes
> a sequence of 20 patches to reach the written-from-scratch stage, then
> fine... but that means each step has been reviewable as you go along.

Nonsense.  If the new code is cleaner and shorter than the existing
code for pools, diffs won't make it any easier to understand.  Our
process cannot be allowed to get in the way of fundamental improvements
to the code base.

....Roy


Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> For the pools code, it can only be patched. It is unacceptable to toss a
> completely written-from-scratch replacement into the code base. If it takes
> a sequence of 20 patches to reach the written-from-scratch stage, then
> fine... but that means each step has been reviewable as you go along.

Nonsense.  If the new code is cleaner and shorter than the existing
code for pools, diffs won't make it any easier to understand.  Our
process cannot be allowed to get in the way of fundamental improvements
to the code base.

....Roy


RE: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
> On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>>...
>> Mine takes it a bit further.  It essentially makes the freelist more
>> efficient.  Adds the possibility to request a new freelist for a pool,
>> which can optionally have locking.
> 
> Cool.
> 
>> Furthermore it resolves some
>> issues with possible segfaults in the current pools code when running
>> out of memory.
> 
> Not necessary :-)

Well, for the apr_pvsprintf routine I disagree.  When out of mem, this
routine will segfault regardless of having registered an abort function
with the pool or not.  In the modified situation, the pools abort
function is called when out of mem and NULL is returned.  I know you
consider the latter unnecessary, but I try to think of other applications
using APR and thus pools which do not register an abort.  And to my
current knowledge even httpd doesn't do so.

Sander


RE: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Sander Striker <st...@apache.org>.
> On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>>...
>> Mine takes it a bit further.  It essentially makes the freelist more
>> efficient.  Adds the possibility to request a new freelist for a pool,
>> which can optionally have locking.
> 
> Cool.
> 
>> Furthermore it resolves some
>> issues with possible segfaults in the current pools code when running
>> out of memory.
> 
> Not necessary :-)

Well, for the apr_pvsprintf routine I disagree.  When out of mem, this
routine will segfault regardless of having registered an abort function
with the pool or not.  In the modified situation, the pools abort
function is called when out of mem and NULL is returned.  I know you
consider the latter unnecessary, but I try to think of other applications
using APR and thus pools which do not register an abort.  And to my
current knowledge even httpd doesn't do so.

Sander


Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>...
> Mine takes it a bit further.  It essentially makes the freelist more
> efficient.  Adds the possibility to request a new freelist for a pool,
> which can optionally have locking.

Cool.

> Furthermore it resolves some
> issues with possible segfaults in the current pools code when running
> out of memory.

Not necessary :-)

> However, there is a downside.  I started with an empty slate and then
> started to add things in.  I still haven't merged some things over
> (the APR_POOL_DEBUG mode, USE_MALLOC and other fancy debug stuff).  Also,
> my code isn't documented very well at the moment (OTOH, the current
> pools code isn't exactly a quick read with all those #if[def] lines...).

For the pools code, it can only be patched. It is unacceptable to toss a
completely written-from-scratch replacement into the code base. If it takes
a sequence of 20 patches to reach the written-from-scratch stage, then
fine... but that means each step has been reviewable as you go along.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [Fwd: brianp patch Quantify results] (was thread-specific freelistfor pools" patch )

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Aug 24, 2001 at 11:09:13PM +0200, Sander Striker wrote:
>...
> Mine takes it a bit further.  It essentially makes the freelist more
> efficient.  Adds the possibility to request a new freelist for a pool,
> which can optionally have locking.

Cool.

> Furthermore it resolves some
> issues with possible segfaults in the current pools code when running
> out of memory.

Not necessary :-)

> However, there is a downside.  I started with an empty slate and then
> started to add things in.  I still haven't merged some things over
> (the APR_POOL_DEBUG mode, USE_MALLOC and other fancy debug stuff).  Also,
> my code isn't documented very well at the moment (OTOH, the current
> pools code isn't exactly a quick read with all those #if[def] lines...).

For the pools code, it can only be patched. It is unacceptable to toss a
completely written-from-scratch replacement into the code base. If it takes
a sequence of 20 patches to reach the written-from-scratch stage, then
fine... but that means each step has been reviewable as you go along.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: [patch] apxs

Posted by Ryan Bloom <rb...@covalent.net>.
Committed.  Thanks.

Ryan

On Monday 27 August 2001 03:03, Stas Bekman wrote:
> I've started from the point where apxs was failing with the error:
>
> /home/stas/httpd-2.0/bin/apxs
> apxs:Error: Invalid query string `SHLTCFLAGS'
>
> because my httpd-2.0/build/config_vars.mk had:
> SHLTCFLAGS =
>
> I was fixing it and while doing that I've refactored some of the code:
>
> So this patch
> - allows empty $val from httpd-2.0/build/config_vars.mk
> - make code more perlish.
> - read and parse the config file only once
> - use a hash instead of array for the list of internal config vars to
>   test against
> - added -h option (just print usage)
> - wrapped numerous print STDERR calls into a simpler error() and
>   notice() subs
> - simplified some of the logic in if's using perl constructs.
>
> I didn't know how to extensively test whether I didn't break anything.
>
> BTW, according to the apxs manpage, the following command
> % /home/stas/httpd-2.0/bin/apxs -q -S PREFIX=bar INCLUDEDIR
> should print:
>
> /bar/include
>
> while it prints:
>
> /home/stas/httpd-2.0/include
>
> which seems to be wrong. This happens in the current apxs version, my
> patch doesn't change it.
>

______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

[patch] apxs

Posted by Stas Bekman <st...@stason.org>.
I've started from the point where apxs was failing with the error:

/home/stas/httpd-2.0/bin/apxs
apxs:Error: Invalid query string `SHLTCFLAGS'

because my httpd-2.0/build/config_vars.mk had:
SHLTCFLAGS =

I was fixing it and while doing that I've refactored some of the code:

So this patch
- allows empty $val from httpd-2.0/build/config_vars.mk
- make code more perlish.
- read and parse the config file only once
- use a hash instead of array for the list of internal config vars to
  test against
- added -h option (just print usage)
- wrapped numerous print STDERR calls into a simpler error() and
  notice() subs
- simplified some of the logic in if's using perl constructs.

I didn't know how to extensively test whether I didn't break anything.

BTW, according to the apxs manpage, the following command
% /home/stas/httpd-2.0/bin/apxs -q -S PREFIX=bar INCLUDEDIR
should print:

/bar/include

while it prints:

/home/stas/httpd-2.0/include

which seems to be wrong. This happens in the current apxs version, my
patch doesn't change it.


Index: apxs.in
===================================================================
RCS file: /home/cvspublic/httpd-2.0/support/apxs.in,v
retrieving revision 1.23
diff -u -r1.23 apxs.in
--- apxs.in	2001/08/26 16:28:19	1.23
+++ apxs.in	2001/08/27 09:56:52
@@ -1,4 +1,4 @@
-#!@perlbin@
+#!@perlbin@ -w
 # ====================================================================
 # The Apache Software License, Version 1.1
 #
@@ -62,7 +62,12 @@
 ##

 my $prefix         = "@prefix@";
-my $CFG_PREFIX     = "$prefix";
+my $CFG_PREFIX     = $prefix;
+
+# read the configuration variables once
+my %config_vars = ();
+get_config_vars("$prefix/build/config_vars.mk",\%config_vars);
+
 my $exec_prefix    = get_vars("exec_prefix");
 my $CFG_TARGET     = get_vars("progname");
 my $CFG_SYSCONFDIR = get_vars("sysconfdir");
@@ -72,9 +77,13 @@
 my $CFG_CC         = get_vars("CC");
 my $libexecdir     = get_vars("libexecdir");
 my $CFG_LIBEXECDIR = eval qq("$libexecdir");
-my $bindir        = get_vars("bindir");
+my $bindir         = get_vars("bindir");
 my $CFG_SBINDIR    = eval qq("$bindir");

+my %internal_vars = map {$_ => 1}
+    qw(TARGET CC CFLAGS CFLAGS_SHLIB LD_SHLIB LDFLAGS_SHLIB LIBS_SHLIB
+       PREFIX SBINDIR INCLUDEDIR LIBEXECDIR SYSCONFDIR);
+
 ##
 ##  parse argument line
 ##
@@ -95,43 +104,43 @@
 my $opt_a = 0;
 my $opt_A = 0;
 my $opt_q = 0;
+my $opt_h = 0;

 #   this subroutine is derived from Perl's getopts.pl with the enhancement of
 #   the "+" metacharater at the format string to allow a list to be build by
 #   subsequent occurance of the same option.
 sub Getopts {
     my ($argumentative, @ARGV) = @_;
-    my (@args, $first, $rest, $pos);
-    my ($errs) = 0;
-    local ($_);
-    local ($[) = 0;
-
-    @args = split( / */, $argumentative);
-    while(@ARGV && ($_ = $ARGV[0]) =~ /^-(.)(.*)/) {
-        ($first, $rest) = ($1,$2);
+    my $errs = 0;
+    local $_;
+    local $[ = 0;
+
+    my @args = split / */, $argumentative;
+    while (@ARGV && ($_ = $ARGV[0]) =~ /^-(.)(.*)/) {
+        my ($first, $rest) = ($1,$2);
         if ($_ =~ m|^--$|) {
-            shift(@ARGV);
+            shift @ARGV;
             last;
         }
-        $pos = index($argumentative,$first);
-        if($pos >= $[) {
-            if($args[$pos+1] eq ':') {
-                shift(@ARGV);
-                if($rest eq '') {
+        my $pos = index($argumentative,$first);
+        if ($pos >= $[) {
+            if ($args[$pos+1] eq ':') {
+                shift @ARGV;
+                if ($rest eq '') {
                     unless (@ARGV) {
-                        print STDERR "apxs:Error: Incomplete option: $first (needs an argument)\n";
-                        ++$errs;
+                        error("Incomplete option: $first (needs an argument)");
+                        $errs++;
                     }
                     $rest = shift(@ARGV);
                 }
                 eval "\$opt_$first = \$rest;";
             }
             elsif ($args[$pos+1] eq '+') {
-                shift(@ARGV);
-                if($rest eq '') {
+                shift @ARGV;
+                if ($rest eq '') {
                     unless (@ARGV) {
-                        print STDERR "apxs:Error: Incomplete option: $first (needs an argument)\n";
-                        ++$errs;
+                        error("Incomplete option: $first (needs an argument)");
+                        $errs++;
                     }
                     $rest = shift(@ARGV);
                 }
@@ -139,7 +148,7 @@
             }
             else {
                 eval "\$opt_$first = 1";
-                if($rest eq '') {
+                if ($rest eq '') {
                     shift(@ARGV);
                 }
                 else {
@@ -148,9 +157,9 @@
             }
         }
         else {
-            print STDERR "apxs:Error: Unknown option: $first\n";
-            ++$errs;
-            if($rest ne '') {
+            error("Unknown option: $first");
+            $errs++;
+            if ($rest ne '') {
                 $ARGV[0] = "-$rest";
             }
             else {
@@ -162,45 +171,47 @@
 }

 sub usage {
-    print STDERR "Usage: apxs -g [-S <var>=<val>] -n <modname>\n";
-    print STDERR "       apxs -q [-S <var>=<val>] <query> ...\n";
-    print STDERR "       apxs -c [-S <var>=<val>] [-o <dsofile>] [-D <name>[=<value>]]\n";
-    print STDERR "               [-I <incdir>] [-L <libdir>] [-l <libname>] [-Wc,<flags>]\n";
-    print STDERR "               [-Wl,<flags>] <files> ...\n";
-    print STDERR "       apxs -i [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...\n";
-    print STDERR "       apxs -e [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...\n";
-    exit(1);
+    print STDERR <<"__USAGE__";
+Usage: apxs -g [-S <var>=<val>] -n <modname>
+       apxs -q [-S <var>=<val>] <query> ...
+       apxs -c [-S <var>=<val>] [-o <dsofile>] [-D <name>[=<value>]]
+               [-I <incdir>] [-L <libdir>] [-l <libname>] [-Wc,<flags>]
+               [-Wl,<flags>] <files> ...
+       apxs -i [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...
+       apxs -e [-S <var>=<val>] [-a] [-A] [-n <modname>] <dsofile> ...
+       apxs -h
+__USAGE__
+    exit 1;
 }

 #   option handling
-my $rc;
-($rc, @ARGV) = &Getopts("qn:gco:I+D+L+l+W+S+eiaA", @ARGV);
-&usage if ($rc == 0);
-&usage if ($#ARGV == -1 and not $opt_g);
-&usage if (not $opt_q and not ($opt_g and $opt_n) and not $opt_i and not $opt_c and not $opt_e);
+my ($rc, @args) = Getopts("qhn:gco:I+D+L+l+W+S+eiaA", @ARGV);
+
+usage() if $opt_h;
+usage() unless $rc;
+usage() unless scalar @args or $opt_g;
+usage() unless $opt_q or ($opt_g and $opt_n) or $opt_i or $opt_c or $opt_e;

 #   argument handling
-my @args = @ARGV;
 my $name = 'unknown';
-$name = $opt_n if ($opt_n ne '');
+$name = $opt_n if $opt_n ne '';

 if (@opt_S) {
-    my ($opt_S);
+    my $opt_S;
     foreach $opt_S (@opt_S) {
 	if ($opt_S =~ m/^([^=]+)=(.*)$/) {
-	    my ($var) = $1;
-	    my ($val) = $2;
-	    my $oldval = eval "\$CFG_$var";
-
-	    unless ($var and $oldval) {
-		print STDERR "apxs:Error: no config variable $var\n";
-		&usage;
+            my ($var) = $1;
+            my ($val) = $2;
+            my $oldval = eval "\$CFG_$var";
+
+            unless ($oldval) {
+		error("no config variable CFG_$var");
+		usage();
 	    }
-
-	    eval "\$CFG_${var}=\"${val}\"";
+            eval "\$CFG_${var}=\"${val}\"";
 	} else {
-	    print STDERR "apxs:Error: malformatted -S option\n";
-	    &usage;
+	    error("malformatted -S option");
+	    usage();
 	}
     }
 }
@@ -208,68 +219,70 @@
 ##
 ##  Initial shared object support check
 ##
-my $exec_prefix = get_vars("exec_prefix");
 my $httpd = get_vars("bindir") . "/" . get_vars("progname");
-my $temp = eval qq("$httpd");
-my $httpd = eval qq("$temp");
+$httpd = eval qq("$httpd");
+$httpd = eval qq("$httpd");

 #allow apxs to be run from the source tree, before installation
 if ($0 =~ m:support/apxs$:) {
     ($httpd = $0) =~ s:support/apxs$::;
 }

-if (not -x "$httpd") {
-	print STDERR "apxs:Error: $httpd not found or not executable\n";
-	exit(1);
-}
-if (not grep(/mod_so/, `$httpd -l`)) {
-    print STDERR "apxs:Error: Sorry, no shared object support for Apache\n";
-    print STDERR "apxs:Error: available under your platform. Make sure\n";
-    print STDERR "apxs:Error: the Apache module mod_so is compiled into\n";
-    print STDERR "apxs:Error: your server binary `$httpd'.\n";
-    exit(1);
+unless (-x "$httpd") {
+	error("$httpd not found or not executable");
+	exit 1;
 }

+unless (grep /mod_so/, `$httpd -l`) {
+    error("Sorry, no shared object support for Apache");
+    error("available under your platform. Make sure");
+    error("the Apache module mod_so is compiled into");
+    error("your server binary `$httpd'.");
+    exit 1;
+}
+
+sub get_config_vars{
+    my ($file, $rh_config) = @_;
+
+    open IN, $file or die "cannot open $file: $!";
+    while (<IN>){
+        if (/^\s*(.*?)\s*=\s*(.*)$/){
+            $rh_config->{$1} = $2;
+        }
+    }
+    close IN;
+}
+
 sub get_vars {
     my $result = '';
-    my $arg;
     my $ok = 0;
+    my $arg;
     foreach $arg (@_) {
-        open IN, "$prefix/build/config_vars.mk" or die "open $prefix/build/config_vars.mk: $!";
-        while (<IN>) {
-            my $var;
-            my $val;
-            if (/(.*) = (.*)$/) {
-                $var = $1;
-                $val = $2;
-            }
-            next unless $var;
-            if ($arg eq $var or $arg eq lc($var)) {
-                $result .= "$val;;";
-                $ok = 1;
-                last;
-            }
+        if (exists $config_vars{$arg} or exists $config_vars{lc $arg}) {
+            my $val = exists $config_vars{$arg}
+                ? $config_vars{$arg}
+                : $config_vars{lc $arg};
+            $result .= eval qq("$val");
+            $result .= ";;";
+            $ok = 1;
         }
         if (not $ok) {
-            foreach $name (qw(
-                TARGET CC CFLAGS CFLAGS_SHLIB LD_SHLIB LDFLAGS_SHLIB LIBS_SHLIB
-                PREFIX SBINDIR INCLUDEDIR LIBEXECDIR SYSCONFDIR
-                )) {
-                if ($arg eq $name or $arg eq lc($name)) {
-                    my $val = eval "\$CFG_$name";
-                    $result .= eval qq("${val}") . ";;";
-                    $ok = 1;
-                }
+            if (exists $internal_vars{$arg} or exists $internal_vars{lc $arg}) {
+                my $val = exists $internal_vars{$arg} ? $arg : lc $arg;
+                $val = eval "\$CFG_$val";
+                $result .= eval qq("$val");
+                $result .= ";;";
+                $ok = 1;
             }
             if (not $ok) {
-                printf(STDERR "apxs:Error: Invalid query string `%s'\n", $arg);
+                error("Invalid query string `$arg'");
                 exit(1);
             }
         }
     }
     $result =~ s|;;$||;
     $result =~ s|:| |;
-    return("$result");
+    return $result;
 }

 ##
@@ -283,11 +296,11 @@
     my ($cmd, $rc);

     foreach $cmd (@cmds) {
-        print STDERR "$cmd\n";
-        $rc = system("$cmd");
-        if ($rc != 0) {
-            printf(STDERR "apxs:Break: Command failed with rc=%d\n", $rc << 8);
-            exit(1);
+        notice($cmd);
+        $rc = system $cmd;
+        if ($rc) {
+            error(sprintf "Command failed with rc=%d\n", $rc << 8);
+            exit 1 ;
         }
     }
 }
@@ -298,7 +311,7 @@
     ##

     if (-d $name) {
-        print STDERR "apxs:Error: Directory `$name' already exists. Remove first\n";
+        error("Directory `$name' already exists. Remove first");
         exit(1);
     }

@@ -308,21 +321,21 @@

     my ($mkf, $mods, $src) = ($data =~ m|^(.+)-=#=-\n(.+)-=#=-\n(.+)|s);

-    print STDERR "Creating [DIR]  $name\n";
+    notice("Creating [DIR]  $name");
     system("mkdir $name");
-    print STDERR "Creating [FILE] $name/Makefile\n";
+    notice("Creating [FILE] $name/Makefile");
     open(FP, ">${name}/Makefile") || die;
     print FP $mkf;
     close(FP);
-    print STDERR "Creating [FILE] $name/modules.mk\n";
+    notice("Creating [FILE] $name/modules.mk");
     open(FP, ">${name}/modules.mk") || die;
     print FP $mods;
     close(FP);
-    print STDERR "Creating [FILE] $name/mod_$name.c\n";
+    notice("Creating [FILE] $name/mod_$name.c");
     open(FP, ">${name}/mod_${name}.c") || die;
     print FP $src;
     close(FP);
-    print STDERR "Creating [FILE] $name/.deps\n";
+    notice("Creating [FILE] $name/.deps");
     system("touch ${name}/.deps");

     exit(0);
@@ -448,7 +461,7 @@
     my $f;
     foreach $f (@args) {
         if ($f !~ m#(\.so$|\.la$)#) {
-            print STDERR "apxs:Error: file $f is not a shared object\n";
+            error("file $f is not a shared object");
             exit(1);
         }
         my $t = $f;
@@ -482,8 +495,8 @@
                 }
             }
             if ($name eq '') {
-                print "apxs:Error: Sorry, cannot determine bootstrap symbol name\n";
-                print "apxs:Error: Please specify one with option `-n'\n";
+                error("Sorry, cannot determine bootstrap symbol name");
+                error("Please specify one with option `-n'");
                 exit(1);
             }
         }
@@ -504,7 +517,7 @@
     #   activate module via LoadModule/AddModule directive
     if ($opt_a or $opt_A) {
         if (not -f "$CFG_SYSCONFDIR/$CFG_TARGET.conf") {
-            print "apxs:Error: Config file $CFG_SYSCONFDIR/$CFG_TARGET.conf not found\n";
+            error("Config file $CFG_SYSCONFDIR/$CFG_TARGET.conf not found");
             exit(1);
         }

@@ -514,8 +527,8 @@
         close(FP);

         if ($content !~ m|\n#?\s*LoadModule\s+|) {
-            print STDERR "apxs:Error: Activation failed for custom $CFG_SYSCONFDIR/$CFG_TARGET.conf file.\n";
-            print STDERR "apxs:Error: At least one `LoadModule' directive already has to exist.\n";
+            error("Activation failed for custom $CFG_SYSCONFDIR/$CFG_TARGET.conf file.");
+            error("At least one `LoadModule' directive already has to exist.");
             exit(1);
         }

@@ -530,7 +543,7 @@
                  $content =~ s|^(.*\n)#?\s*$lmd[^\n]*\n|$1$c$lmd\n|sg;
             }
             $lmd =~ m|LoadModule\s+(.+?)_module.*|;
-            print STDERR "[$what module `$1' in $CFG_SYSCONFDIR/$CFG_TARGET.conf]\n";
+            notice("[$what module `$1' in $CFG_SYSCONFDIR/$CFG_TARGET.conf]");
         }
         my $amd;
         foreach $amd (@amd) {
@@ -548,10 +561,18 @@
                        "cp $CFG_SYSCONFDIR/$CFG_TARGET.conf.new $CFG_SYSCONFDIR/$CFG_TARGET.conf && " .
                        "rm $CFG_SYSCONFDIR/$CFG_TARGET.conf.new");
             } else {
-                print STDERR "unable to open configuration file\n";
+                notice("unable to open configuration file");
             }
 	}
     }
+}
+
+sub error{
+    print STDERR "apxs:Error: $_[0].\n";
+}
+
+sub notice{
+    print STDERR "$_[0]\n";
 }

 ##EOF##

_____________________________________________________________________
Stas Bekman              JAm_pH     --   Just Another mod_perl Hacker
http://stason.org/       mod_perl Guide  http://perl.apache.org/guide
mailto:stas@stason.org   http://localhost/      http://eXtropia.com/
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/