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" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
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
>
