Compilation with gcc 4.6.0

[Julien ÉLIE]

Hi Russ,

> I suspect there are latent stack handling bugs in PERLfilter should the
> stack ever end up getting reallocated.

Yep, you're totally right.
I believe it is what I experienced yesterday when innd was segfaulting. 
  The Perl stack may have been reallocated or corrupted.

A copy of the Perl stack pointer is saved at several places in the code 
but not always restored the way it should be.
In PLartfilter() in innd/perl.c, we have the following calls:

   ENTER;
   SAVETMPS;
   PUSHMARK(SP);
   rc = perl_call_sv((SV *) filter, G_EVAL|G_SCALAR|G_NOARGS);
   SPAGAIN;
   if (SvTRUE(ERRSV)) {
     (void) POPs;
     PerlFilter(false);
   }
   PUTBACK;
   FREETMPS;
   LEAVE;

And PerlFilter(), as you mentioned, also plays with the stack:

   ENTER;
   SAVETMPS;
   perl_call_argv("filter_end", G_EVAL | G_DISCARD | G_NOARGS, argv);
   (void) POPs;
   FREETMPS;
   LEAVE;




I refactored PLartfilter() to use something like:

+ bool failure = false;
   ENTER;
   SAVETMPS;
   PUSHMARK(SP);
+ PUTBACK;
   rc = perl_call_sv((SV *) filter, G_EVAL|G_SCALAR|G_NOARGS);
   SPAGAIN;
   if (SvTRUE(ERRSV)) {
+   failure = true;
     (void) POPs;
-   PerlFilter(false);
   }
   PUTBACK;
   FREETMPS;
   LEAVE;
+ if (failure) {
+   PerlFilter(false);
+ }


It seems to do the trick.


Full commit here:
     http://inn.eyrie.org/trac/changeset/9205/trunk





> There is a lot of code like:
>
>      if (perl_get_cv("filter_before_reload", false) != NULL)    {
>          perl_call_argv("filter_before_reload", G_EVAL|G_DISCARD|G_NOARGS, argv);
>          if (SvTRUE(ERRSV))     /* check $@ */ {
>              syslog (L_ERROR,"SERVER perl function filter_before_reload died: %s",
>                      SvPV(ERRSV, PL_na)) ;
>              (void)POPs ;
>              PerlFilter (false) ;
>          }
>      }
>
> in that function that I think, according to perlcall, should be surrounded
> in similar macro calls.

Yes.
I have found extensive, detailed examples in:
     http://docstore.mik.ua/orelly/perl/advprog/ch20_04.htm
     http://www.perlmonks.org/?node_id=830663
     http://dev.csoft.org/mailprocd/sa.c




> There are deeper problems with all that code, though, namely that it uses
> a rather awkward way to load code, and the way it runs the filter code is
> even worse.

It is less urgent to change, now that I believe the Perl stack is 
handled by the right macros everywhere it should.

Of course, do not hesitate to tell if you see something wrong in the commit.

-- 
Julien ÉLIE

« Vita breuis, ars longa, occasio praeceps, experimentum pericolosum,
   iudicium difficile. » (Hippocrate)