Compilation with gcc 4.6.0

[Russ Allbery]

Julien ÉLIE <julien at trigofacile.com> writes:

>>> static bool
>>> ListHas(const char **list, const char *p)
>>> {
>>>    const char	*q;
>>>    char		c;
>>>
>>>    for (c = *p; (q = *list) != NULL; list++)
>>>      if (strcasecmp(p, q) == 0)
>>>        return true;
>>>    return false;
>>
>> That whole function is just weirdly written.  I'd write this as:
>>
>>      size_t i;
>>
>>      for (i = 0; list[i] != NULL; i++)
>>          if (strcasecmp(p, list[i]) == 0)
>>              return true;
>>      return false;

> Your suggestion is easier to read, I agree.  Yet, it adds more
> computations (if i=100, you will have to walk to list[100] twice, and so
> on with 101, 102, etc.).  The complexity of the function is now O(n^2)
> whereas it was O(n) with list++ :-)

I'm not following.  list is just an array of strings.  Previously, we
traversed the array via incrementing a pointer; in the new version, we
traverse the array using an explicit offset.  But either way, the
generated assembly code should amount to functionally the same thing (in
fact, an optimizing compiler would probably generate nearly identical
code for both versions).

>> This part is just a mess and has needed rewriting for a while.  The
>> real work that requires that bracketing is happening in PERLfilter, so

Sorry, this should have been PERLreadfilter.

>> really the stack manipulation should probably be moved into it.  The
>> above probably doesn't do any harm, but I don't think it catches all
>> the weirdness that we should be handling.

> When you speak about "all the weirdness that we should be handling", do
> you hint at peculiar (known) bugs?  or just at the fact it needs
> rewriting for clarity?

I suspect there are latent stack handling bugs in PERLfilter should the
stack ever end up getting reallocated.  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 fuction that I think, according to perlcall, should be surrounded
in similar macro calls.  I'm not sure about the perl_eval_pv() calls,
although I suspect they may do the same thing.

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.  Once upon a time, I had started figuring out what the "right"
way to do this probably would be, and then never finished.  :/ PerlSilence
is weird and needs a better solution, and I think there were native
functions that did what we're doing with perl_eval_pv() to load Perl code
from a file.  And there's a bunch of code embedded in innd and nnrpd that
should be lifted into a more generic filter layer.

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>

    Please send questions to the list rather than mailing me directly.
     <http://www.eyrie.org/~eagle/faqs/questions.html> explains why.