Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <47EFA56C.5090701@apache.org>
Date: Sun, 30 Mar 2008 16:36:28 +0200
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.13) Gecko/20080313 SeaMonkey/1.1.9
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r642558 - in /httpd/httpd/trunk: CHANGES
 NWGNUmakefile
 build/nw_export.inc include/ap_expr.h include/ap_mmn.h libhttpd.dsp
 server/Makefile.in
 server/util_expr.c
References: <20080329171338.6E6191A9832@eris.apache.org>
In-Reply-To: <20080329171338.6E6191A9832@eris.apache.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit


On 03/29/2008 06:13 PM, niq@apache.org wrote:
> Author: niq
> Date: Sat Mar 29 10:13:28 2008
> New Revision: 642558
> 
> URL: http://svn.apache.org/viewvc?rev=642558&view=rev
> Log:
> Introduce ap_expr expression parser API
> 
> Added:
>     httpd/httpd/trunk/include/ap_expr.h
>     httpd/httpd/trunk/server/util_expr.c
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/NWGNUmakefile
>     httpd/httpd/trunk/build/nw_export.inc
>     httpd/httpd/trunk/include/ap_mmn.h
>     httpd/httpd/trunk/libhttpd.dsp
>     httpd/httpd/trunk/server/Makefile.in
> 

> Added: httpd/httpd/trunk/server/util_expr.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_expr.c?rev=642558&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/server/util_expr.c (added)
> +++ httpd/httpd/trunk/server/util_expr.c Sat Mar 29 10:13:28 2008

> +#define PARSE_STRING(r,s) (string_func ? string_func((r),(s)) : (s))
> +AP_DECLARE(int) ap_expr_eval(request_rec *r, parse_node_t *root,
> +                             int *was_error, backref_t **reptr,
> +                             string_func_t string_func, opt_func_t eval_func)
> +{
> +    parse_node_t *current = root;
> +    const char *error = NULL;
> +    unsigned int regex = 0;
> +
> +    /* Evaluate Parse Tree */
> +    while (current) {
> +        switch (current->token.type) {
> +        case TOKEN_STRING:
> +            current->token.value = PARSE_STRING(r, current->token.value);
> +            current->value = !!*current->token.value;
> +            break;
> +
> +        case TOKEN_AND:
> +        case TOKEN_OR:
> +            if (!current->left || !current->right) {
> +                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
> +                              "Invalid expression in file %s", r->filename);
> +                *was_error = 1;
> +                return 0;
> +            }
> +
> +            if (!current->left->done) {
> +                switch (current->left->token.type) {
> +                case TOKEN_STRING:
> +                    current->left->token.value =
> +                        PARSE_STRING(r, current->left->token.value);
> +                    current->left->value = !!*current->left->token.value;

Why do we use !! here? Isn't this the same as !! not being there?


> +                    DEBUG_DUMP_EVAL(ctx, current->left);
> +                    current->left->done = 1;
> +                    break;
> +
> +                default:
> +                    current = current->left;
> +                    continue;
> +                }
> +            }
> +
> +            /* short circuit evaluation */
> +            if (!current->right->done && !regex &&

I don't get the purpose of regex here. Why can't we short circuit if regex !=0?

> +                ((current->token.type == TOKEN_AND && !current->left->value) ||
> +                (current->token.type == TOKEN_OR && current->left->value))) {
> +                current->value = current->left->value;
> +            }
> +            else {
> +                if (!current->right->done) {
> +                    switch (current->right->token.type) {
> +                    case TOKEN_STRING:
> +                        current->right->token.value =
> +                            PARSE_STRING(r,current->right->token.value);
> +                        current->right->value = !!*current->right->token.value;

same as above.

General question: I know that this parser was taken from mod_include and I haven't
really dived into its details, but wouldn't it be better and faster for evaluation
to build UPN stack from the expression instead of the parser tree?

Regards

Rüdiger