On Thu, 26 Jul 2007 22:08:10 +0200 Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> > - else if (!provider->match.string) { > > - match = 0; > > - } Lines removed; it's the replacement of those later you're questioning. > > + /* we can't check for NULL in provider as that kills > > integer 0 > > + * so we have to test each string/regexp case in the switch > > + */ > > else { > > - /* Now we have no nulls, so we can do string and > > regexp matching */ Explanation - I don't recollect it, but I expect I had segfaults at some point in development, and put the check in to prevent them. Or maybe it was just to preempt them. > Hm. How can provider->match.string == NULL if provider->match_type ~ > STRING_MATCH|STRING_CONTAINS? Good question. > Same here: How can provider->match.string = provider->match.regex == > NULL if provider->match_type == REGEX_MATCH? If ap_pregcomp fails in > filter_provider we should bail out there IMHO. Agreed. I'm happy with the changes you imply in this and your other post. On the other hand, removing the check might risk segfaulting in some future update - perhaps something that gets configuration per-request from a database and a rewritemap. I've recently had a similar issue with another module, which started life using match rules defined in httpd.conf, then grew to derive them from rewritemap+dbm. Worked fine until it constructed a match-rule from a corrupted database entry. -- Nick Kew Application Development with Apache - the Apache Modules Book http://www.apachetutor.org/