httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject [PATCH] I prefer this... Re: [PATCH] for mod_mime seg fault in 2.0.23 :-)
Date Tue, 14 Aug 2001 20:57:39 GMT
I prefer this patch which creates a completely new, fully merged, mime_dir_config on each
call to merge_mime_dir_configs. It is a bit more expensive but should it still work better
than the table code (maybe. I'd like to see it profiled :-). And this code is a hell of a
lot easier to understand :-)

Bill

Index: mod_mime.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
retrieving revision 1.50
diff -u -r1.50 mod_mime.c
--- mod_mime.c 2001/08/14 02:35:55 1.50
+++ mod_mime.c 2001/08/14 20:46:56
@@ -105,14 +105,11 @@
     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 */
@@ -150,7 +147,6 @@
     (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;
@@ -180,13 +176,10 @@
         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;
-            }
+            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));

             if (overlay_info->forced_type) {
                 base_info->forced_type = overlay_info->forced_type;
@@ -219,36 +212,15 @@
     int i;
     attrib_info *suffix;

-    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);
-            overlay_extension_mappings(p, base->extension_mappings,
-                                       new->extension_mappings);
-        }
-        overlay_extension_mappings(p, add->extension_mappings,
-                                   new->extension_mappings);
-        new->copy_mappings = 1;
+    new->extension_mappings = apr_hash_make(p);
+
+    if (base->extension_mappings != NULL) {
+        overlay_extension_mappings(p, base->extension_mappings,
+                                   new->extension_mappings);
     }
-    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);
-            /* ### 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->extension_mappings != NULL) {
+        overlay_extension_mappings(p, add->extension_mappings,
+                                   new->extension_mappings);
     }

     if (add->handlers_remove) {
@@ -259,14 +231,6 @@
                                               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;
             }
         }
@@ -281,14 +245,6 @@
                                               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;
             }
         }
@@ -303,14 +259,6 @@
                                               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;
             }
         }
@@ -792,6 +740,7 @@
     char *ext;
     const char *fn, *type, *charset = NULL;
     int found_metadata = 0;
+    extension_info *exinfo = NULL;

     if (r->finfo.filetype == APR_DIR) {
         r->content_type = DIR_MAGIC_TYPE;
@@ -820,7 +769,7 @@
     /* Parse filename extensions which can be in any order
      */
     while (*fn && (ext = ap_getword(r->pool, &fn, '.'))) {
-        extension_info *exinfo;
+
         int found;

         if (*ext == '\0')  /* ignore empty extensions "bad..html" */


----- Original Message -----
From: "Greg Ames" <gregames@remulak.net>
To: <new-httpd@apache.org>
Sent: Tuesday, August 14, 2001 3:50 PM
Subject: [PATCH] for mod_mime seg fault in 2.0.23 :-)


> Bill Stoddard wrote:
> >
>
> > The problem is that we are creating an extension_info out of a subrequest pool and
> > sticking it into a longer lived hash table (allocated out of the request pool I'm
> > guessing). When we pull the extension_info out of the hash table later and try to
use
it,
> > we seg fault because the storage has been reused for something else. No thoughts
on a
fix.
>
> This fixes the seg fault on my Linux box:
>
> Index: modules/http/mod_mime.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
> retrieving revision 1.50
> diff -u -d -b -r1.50 mod_mime.c
> --- modules/http/mod_mime.c     2001/08/14 02:35:55     1.50
> +++ modules/http/mod_mime.c     2001/08/14 19:39:45
> @@ -220,13 +220,10 @@
>      attrib_info *suffix;
>
>      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);
>              overlay_extension_mappings(p, base->extension_mappings,
>                                         new->extension_mappings);
> -        }
> +
>          overlay_extension_mappings(p, add->extension_mappings,
>                                     new->extension_mappings);
>
> We can't allow overlay_extension_mappings to store things into the
> extension_mappings hash table which have a shorter lifetime than the
> hash table itself.  Always using a new extension_mappings prevents this
> from happening.
>
> I'll commit this in a little while unless someone objects soon.
>
> Greg
> Greg
>


Mime
View raw message