You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2001/08/02 01:33:08 UTC

cvs commit: httpd-2.0/modules/http mod_mime.c

wrowe       01/08/01 16:33:08

  Modified:    .        CHANGES
               modules/http mod_mime.c
  Log:
    Solve the merge bugs, by storing a copy bit to save us a ton of
    reallocation in the merge config phase.  Should keep most of the
    savings realized by moving to tables.
  
    Need others to vet this code, please!  Look at this and the prior
    patch as a single diff (-r n -r n-2) to see the overall changes.
  
  Revision  Changes    Path
  1.269     +3 -0      httpd-2.0/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/CHANGES,v
  retrieving revision 1.268
  retrieving revision 1.269
  diff -u -r1.268 -r1.269
  --- CHANGES	2001/07/31 13:00:50	1.268
  +++ CHANGES	2001/08/01 23:33:07	1.269
  @@ -1,5 +1,8 @@
   Changes with Apache 2.0.23-dev
   
  +  *) Fix broken mod_mime behavior in merging its arguments.  Possible
  +     cause of unexplicable crashes introduced in 2.0.20.  [William Rowe]
  +
     *) Solve many mod_ssl porting issues (too many to detail) with 
        help from the whole team, but most notably [Ralf S. Engelschall, 
        Madhusudan Mathihalli <ma...@hp.com>, 
  
  
  
  1.44      +72 -14    httpd-2.0/modules/http/mod_mime.c
  
  Index: mod_mime.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
  retrieving revision 1.43
  retrieving revision 1.44
  diff -u -r1.43 -r1.44
  --- mod_mime.c	2001/07/11 04:47:02	1.43
  +++ mod_mime.c	2001/08/01 23:33:08	1.44
  @@ -105,11 +105,14 @@
       char *language_type;              /* Added with AddLanguage... */
       char *handler;                    /* Added with AddHandler... */
       char *charset_type;               /* Added with AddCharset... */
  +    int copy;                         /* To avoid dupping in the merge */
   } extension_info;
   
   typedef struct {
       apr_hash_t  *extension_mappings;  /* Map from extension name to
                                          * extension_info structure */
  +    int copy_mappings;          /* To avoid dupping in the merge */
  +
       apr_array_header_t *handlers_remove;  /* List of handlers to remove */
       apr_array_header_t *types_remove;     /* List of MIME types to remove */
       apr_array_header_t *encodings_remove; /* List of encodings to remove */
  @@ -147,6 +150,8 @@
       (mime_dir_config *) apr_palloc(p, sizeof(mime_dir_config));
   
       new->extension_mappings = apr_hash_make(p);
  +    new->copy_mappings = 0;
  +
       new->handlers_remove = NULL;
       new->types_remove = NULL;
       new->encodings_remove = NULL;
  @@ -171,8 +176,18 @@
           extension_info *overlay_info, *base_info;
           
           apr_hash_this(index, (const void**)&key, &klen, (void**)&overlay_info);
  +
           base_info = (extension_info*)apr_hash_get(base, key, klen);
  +
           if (base_info) {
  +            if (!base_info->copy) {
  +                extension_info *copyinfo = base_info;
  +                base_info = (extension_info*)apr_palloc(p, sizeof(*base_info));
  +                apr_hash_set(base, key, klen, base_info);
  +                memcpy(base_info, copyinfo, sizeof(*base_info));
  +                base_info->copy = 1;
  +            }
  +
               if (overlay_info->forced_type) {
                   base_info->forced_type = overlay_info->forced_type;
               }
  @@ -190,9 +205,7 @@
               }
           }
           else {
  -            base_info = (extension_info*)apr_palloc(p, sizeof(extension_info));
  -            memcpy(base_info, overlay_info, sizeof(extension_info));
  -            apr_hash_set(base, key, klen, base_info);
  +            apr_hash_set(base, key, klen, overlay_info);
           }
       }
   }
  @@ -206,19 +219,38 @@
       int i;
       attrib_info *suffix;
   
  -    if (base->extension_mappings == NULL) {
  -        new->extension_mappings = add->extension_mappings;
  -    }
  -    else if (add->extension_mappings == NULL) {
  -        new->extension_mappings = base->extension_mappings;
  -    }
  -    else {
  -        new->extension_mappings = apr_hash_make(p);
  -        overlay_extension_mappings(p, base->extension_mappings,
  -                                   new->extension_mappings);
  +    if (base->extension_mappings && add->extension_mappings) {
  +        if (base->copy_mappings)
  +            new->extension_mappings = base->extension_mappings;
  +        else {
  +            new->extension_mappings = apr_hash_make(p);
  +            /* XXX as slow as can be... just use an apr_hash_dup! */
  +            overlay_extension_mappings(p, base->extension_mappings,
  +                                       new->extension_mappings);
  +        }
           overlay_extension_mappings(p, add->extension_mappings,
                                      new->extension_mappings);
  +        new->copy_mappings = 1;
       }
  +    else {
  +        if (base->extension_mappings == NULL) {
  +            new->extension_mappings = add->extension_mappings;
  +            new->copy_mappings = add->copy_mappings;
  +        }
  +        else if (add->extension_mappings == NULL) {
  +            new->extension_mappings = base->extension_mappings;
  +            new->copy_mappings = base->copy_mappings;
  +        }
  +        if (!new->copy_mappings && (add->handlers_remove 
  +                                 || add->types_remove 
  +                                 || add->encodings_remove)) {
  +            apr_hash_t *copyhash = new->extension_mappings;
  +            new->extension_mappings = apr_hash_make(p);
  +            /* XXX as slow as can be... just use an apr_hash_dup! */
  +            overlay_extension_mappings(p, copyhash, new->extension_mappings);
  +            new->copy_mappings = 1;
  +        }
  +    }
   
       if (add->handlers_remove) {
           suffix = (attrib_info *) add->handlers_remove->elts;
  @@ -228,6 +260,14 @@
                                                 suffix[i].name,
                                                 APR_HASH_KEY_STRING);
               if (exinfo) {
  +                if (!exinfo->copy) {
  +                    extension_info *copyinfo = exinfo;
  +                    exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
  +                    apr_hash_set(new->extension_mappings,  suffix[i].name, 
  +                                APR_HASH_KEY_STRING, exinfo);
  +                    memcpy(exinfo, copyinfo, sizeof(*exinfo));
  +                    exinfo->copy = 1;
  +                }
                   exinfo->handler = NULL;
               }
           }
  @@ -241,6 +281,14 @@
                                                 suffix[i].name,
                                                 APR_HASH_KEY_STRING);
               if (exinfo) {
  +                if (!exinfo->copy) {
  +                    extension_info *copyinfo = exinfo;
  +                    exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
  +                    apr_hash_set(new->extension_mappings,  suffix[i].name, 
  +                                APR_HASH_KEY_STRING, exinfo);
  +                    memcpy(exinfo, copyinfo, sizeof(*exinfo));
  +                    exinfo->copy = 1;
  +                }
                   exinfo->forced_type = NULL;
               }
           }
  @@ -254,6 +302,14 @@
                                                 suffix[i].name,
                                                 APR_HASH_KEY_STRING);
               if (exinfo) {
  +                if (!exinfo->copy) {
  +                    extension_info *copyinfo = exinfo;
  +                    exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
  +                    apr_hash_set(new->extension_mappings,  suffix[i].name, 
  +                                APR_HASH_KEY_STRING, exinfo);
  +                    memcpy(exinfo, copyinfo, sizeof(*exinfo));
  +                    exinfo->copy = 1;
  +                }
                   exinfo->encoding_type = NULL;
               }
           }
  @@ -266,6 +322,7 @@
   
       return new;
   }
  +
   static extension_info *get_extension_info(apr_pool_t *p, mime_dir_config *m,
                                             const char* key)
   {
  @@ -274,6 +331,7 @@
       exinfo = (extension_info*)apr_hash_get(m->extension_mappings, key,
                                              APR_HASH_KEY_STRING);
       if (!exinfo) {
  +        /* pcalloc sets ->copy to false */
           exinfo = apr_pcalloc(p, sizeof(extension_info));
           apr_hash_set(m->extension_mappings, apr_pstrdup(p, key),
                        APR_HASH_KEY_STRING, exinfo);
  @@ -480,7 +538,7 @@
   
       types_confname = ap_server_root_relative(p, types_confname);
   
  -    if ((status = ap_pcfg_openfile(&f, p, types_confname)) != APR_SUCCESS) {
  +    if ((status = ap_pcfg_openfile(&f, ptemp, types_confname)) != APR_SUCCESS) {
           ap_log_error(APLOG_MARK, APLOG_ERR, status, s,
   		     "could not open mime types config file %s.", types_confname);
           exit(1);
  
  
  

Re: 2.0.23?

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Cliff Woolley" <cl...@yahoo.com>
Sent: Thursday, August 02, 2001 1:51 PM



> > wrowe       01/08/01 12:15:22
> >
> >   Modified:    server   core.c
> >   Log:
> >     This patch fixes more untold breakage than you can shake a stick at.
> 
> APR_BRIGADE_FOREACH(e,b) provides a perfectly usable e as long as you
> don't MODIFY e within the loop, because the foreach gets to the next
> bucket in the brigade using APR_BUCKET_NEXT(e).  I haven't looked yet
> at that loop (but I will)... was e getting modified?  If so, then
> yeah, that would cause loads of funky problems.
> 
> Are there other planned changes to the threaded MPM or to the MPM API (as
> in the current ap_graceful_stop_signalled() discussion)?

No, we were treating 'e' as legit AFTER we had dropped out of the APR_BRIGADE_FOREACH,
this was very broken.


Re: 2.0.23?

Posted by Greg Ames <gr...@remulak.net>.
Cliff Woolley wrote:
> 

> Are there other planned changes to the threaded MPM or to the MPM API (as
> in the current ap_graceful_stop_signalled() discussion)?

I have a quick patch to threaded & worker coming in a sec.  Let's hold
off on changing the API until after the Shiny Beta.

There's another one to threaded to set permissions if the accept mutex
is sysv sem serialized.  But I didn't make any progress on that today,
it won't be APR-ized as it should (i.e. it's a quick hack for now), and
I have band practice tonite.  The patch is fairly important for OS/390,
but I suppose we could patch the tarball after the t&r and document
exactly what we did in the OS/390 binary's README.

Greg

Re: 2.0.23?

Posted by Cliff Woolley <cl...@yahoo.com>.
On 1 Aug 2001 wrowe@apache.org wrote:

> wrowe       01/08/01 16:33:08
>
>   Modified:    .        CHANGES
>                modules/http mod_mime.c
>   Log:
>     Solve the merge bugs, by storing a copy bit to save us a ton of
>     reallocation in the merge config phase.  Should keep most of the
>     savings realized by moving to tables.
>
>     Need others to vet this code, please!  Look at this and the prior
>     patch as a single diff (-r n -r n-2) to see the overall changes.
>

You asked me to hold, so I'm holding.  =-)  How close are we to having
this licked after this patch?  (I intend to go over all this with a
fine-toothed comb, BTW, but it'll have to wait a few hours.)

This problem aside, what are the other currently known problems?  I think
I heard that the linux build problems were fixed (I will of course
double-check that it works on my RH7.1 box).  Oh, and what's with this
commit?

> wrowe       01/08/01 12:15:22
>
>   Modified:    server   core.c
>   Log:
>     This patch fixes more untold breakage than you can shake a stick at.

APR_BRIGADE_FOREACH(e,b) provides a perfectly usable e as long as you
don't MODIFY e within the loop, because the foreach gets to the next
bucket in the brigade using APR_BUCKET_NEXT(e).  I haven't looked yet
at that loop (but I will)... was e getting modified?  If so, then
yeah, that would cause loads of funky problems.

Are there other planned changes to the threaded MPM or to the MPM API (as
in the current ap_graceful_stop_signalled() discussion)?

Thanks,
Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA