httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <stodd...@raleigh.ibm.com>
Subject Re: {Patch] Port mod_rewrite to use APR for Apache 2.0 (again).
Date Fri, 31 Dec 1999 05:46:02 GMT
I spend about 40 minutes looking at this and it all looks reasonable.



> Index: mod_rewrite.c
> ===================================================================
> RCS file: /home/cvs/apache-2.0/src/modules/standard/mod_rewrite.c,v
>      /* whether proxy module is available or not */
>  static int proxy_available;
> +static int once_through = 0;
Should we do the initialization on the second time through (like
mod_auth_digest?).  Not related to this patch but I also noticed that the
mod_mime & mod_mime_magic modules do not detect if the init phase has
already run.

> -static void rewritelock_remove(void *data)
> +static ap_status_t rewritelock_remove(void *data)
>  {
>      /* only operate if a lockfile is used */
>      if (lockname == NULL || *(lockname) == '\0') {
> -        return;
> +        return(-1);
Humm, shouldn't this be returning an APR_STATUS ?

>      }
>
>      /* remove the lockfile */
>      unlink(lockname);
>      lockname = NULL;
> -    lockfd = -1;
> +    lockfd = NULL;
> +    return(0);
Ditto.


> -        rc = ap_spawn_child(p, rewritemap_program_child,
> -                            (void *)map->datafile, kill_after_timeout,
> -                            &fpin, &fpout, &fperr);
> -        if (rc == 0 || fpin == NULL || fpout == NULL) {
> -            ap_log_error(APLOG_MARK, APLOG_ERR, s,
> +        rc = rewritemap_program_child(p, map->datafile,
> +                                     &fpout, &fpin, &fperr);
> +        if (rc != APR_SUCCESS || fpin == NULL || fpout == NULL) {
> +            ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
>                           "mod_rewrite: could not fork child for "
> -                         "RewriteMap process");
> +                         "RewriteMap process. %d", rc);

<snip...>

>  /* child process code */
> -static int rewritemap_program_child(void *cmd, child_info *pinfo)
> -{
> -    int child_pid = 1;
> +static int rewritemap_program_child(ap_context_t *p, char *progname,
> +                                    ap_file_t **fpout, ap_file_t **fpin,
> +                                    ap_file_t **fperr)
> +{
> +    int rc = -1;
> +    ap_procattr_t *procattr;
> +    ap_proc_t *procnew;
>
> -    /*
> -     * Prepare for exec
> -     */
> -    ap_cleanup_for_exec();
> +    ap_block_alarms();
> +

<snip>

> +
> +    if ((ap_createprocattr_init(&procattr, p)           != APR_SUCCESS)
||
> +        (ap_setprocattr_io(procattr, APR_FULL_BLOCK,
> +                                     APR_FULL_NONBLOCK,
> +                                     APR_FULL_NONBLOCK) != APR_SUCCESS)
||
> +        (ap_setprocattr_dir(procattr, ap_make_dirstr_parent(p, progname))
> +                                                        != APR_SUCCESS)
||
> +        (ap_setprocattr_cmdtype(procattr, APR_PROGRAM)  != APR_SUCCESS))
{
> +        /* Something bad happened, give up and go away. */
> +        rc = -1;
> +    }
> +    else {
> +        rc = ap_create_process(&procnew, progname, NULL, NULL, procattr,
p);
> +
> +        if (rc == APR_SUCCESS) {
> +            ap_note_subprocess(p, procnew, kill_after_timeout);
> +
> +            if (fpin) {
> +                ap_get_childin(fpin, procnew);
> +            }
> +
> +            if (fpout) {
> +                ap_get_childout(fpout, procnew);
> +            }
> +
> +            if (fperr) {
> +                ap_get_childerr(fperr, procnew);
> +            }
> +        }
> +    }
> +
> +    ap_unblock_alarms();
> +
> +    return (rc);
>  }
Didn't look at this in detail, but it looks generally right. I don't think
we need block/unblock alarms. Someone correct me if I am wrong.

I'd like to see this tested (at least compiled) on Windows.  That'll have to
wait till next week when I get the Windows port compiling again. The time
functions are all hosed up.  Unless someone objects, I will commit this
patch to the Apache 2.0 repository tomorrow.

Bill



Mime
View raw message