From: "William A. Rowe, Jr." <[EMAIL PROTECTED]>
Sent: Tuesday, August 14, 2001 9:51 PM
> > OK. More complex, and not 'tweaked' for legibility. Rather than make you all
>wait,
> > I'm posting as-is, and won't commit yet, since we have some contention about the
>'right'
> > solution.
>
> Also introduces or reveals the segfault for me... I'm reviewing now, you might want
>to
> wait just a bit, I'll post a revision in an hour or so.
Sorry. As usual (when you hide things in nasty casts) it pays to test.
Attached is the proper patch. Please, test this against the segfault cases.
Bill
Index: modules/http/mod_mime.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/mod_mime.c,v
retrieving revision 1.51
diff -u -r1.51 mod_mime.c
--- modules/http/mod_mime.c 2001/08/15 03:09:32 1.51
+++ modules/http/mod_mime.c 2001/08/15 04:07:46
@@ -105,13 +105,13 @@
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 */
+ apr_pool_t *pool; /* To dup appropriately 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_pool_t *pool; /* To dup appropriately 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 */
@@ -149,8 +149,8 @@
mime_dir_config *new =
(mime_dir_config *) apr_palloc(p, sizeof(mime_dir_config));
- new->extension_mappings = apr_hash_make(p);
- new->copy_mappings = 0;
+ new->pool = p;
+ new->extension_mappings = NULL;
new->handlers_remove = NULL;
new->types_remove = NULL;
@@ -179,13 +179,20 @@
base_info = (extension_info*)apr_hash_get(base, key, klen);
- if (base_info) {
- if (!base_info->copy) {
+ /* If we don't find the entry, simply add it (as is).
+ * If we find the entry, and this exinfo wasn't allocated from the
+ * current pool, then we must copy before can merge the entry.
+ */
+ if (!base_info) {
+ apr_hash_set(base, key, klen, overlay_info);
+ }
+ else {
+ if (base_info->pool != p) {
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;
+ base_info->pool = p;
}
if (overlay_info->forced_type) {
@@ -204,8 +211,34 @@
base_info->charset_type = overlay_info->charset_type;
}
}
- else {
- apr_hash_set(base, key, klen, overlay_info);
+ }
+}
+
+
+static void remove_items(apr_pool_t *p, apr_array_header_t *remove,
+ apr_hash_t *mappings, apr_size_t member)
+{
+ attrib_info *suffix = (attrib_info *) remove->elts;
+ int i;
+ for (i = 0; i < remove->nelts; i++) {
+ extension_info *exinfo =
+ (extension_info*)apr_hash_get(mappings,
+ suffix[i].name,
+ APR_HASH_KEY_STRING);
+ /* If we find the entry, the given member isn't NULL,
+ * and this exinfo wasn't allocated from the current pool,
+ * then we must copy before we remove the given member.
+ */
+ if (exinfo && *((void **)((char*)&exinfo + member))) {
+ if (exinfo->pool != p) {
+ extension_info *copyinfo = exinfo;
+ exinfo = (extension_info*)apr_palloc(p, sizeof(*exinfo));
+ apr_hash_set(mappings, suffix[i].name,
+ APR_HASH_KEY_STRING, exinfo);
+ memcpy(exinfo, copyinfo, sizeof(*exinfo));
+ exinfo->pool = p;
+ }
+ *(void **)((char*)exinfo + member) = NULL;
}
}
}
@@ -215,106 +248,52 @@
mime_dir_config *base = (mime_dir_config *) basev;
mime_dir_config *add = (mime_dir_config *) addv;
mime_dir_config *new = apr_palloc(p, sizeof(mime_dir_config));
+ extension_info dummyex;
+ new->pool = p;
- int i;
- attrib_info *suffix;
-
- if (base->extension_mappings && add->extension_mappings) {
- /* XXX: Greg Ames' fast hack to always copy the hash.
- * It's incomplete, and slower than tables, but at least
- * the server (mostly) runs once again.
- */
- new->extension_mappings = apr_hash_make(p);
+ /* Given no mappings, we set up no mappings table.
+ * If we are using the same pool as the base cfg, _or_
+ * we neither have anything to add or remove, we
+ * then we are safe retaining the existing hash.
+ * Otherwise we need a private copy of the hash table.
+ */
+ if (!base->extension_mappings && !add->extension_mappings)
+ new->extension_mappings = NULL;
+ else if (base->extension_mappings
+ && ((base->pool == p) || !(add->extension_mappings
+ || add->handlers_remove
+ || add->types_remove
+ || add->encodings_remove)))
+ new->extension_mappings = base->extension_mappings;
+ else {
+ new->extension_mappings = apr_hash_make(p);
+ if (base->extension_mappings)
overlay_extension_mappings(p, base->extension_mappings,
new->extension_mappings);
+ }
+
+ /* Now we can add and remove hash table entries. These functions
+ * are given a 'safe' hash they can modify, but they have to be
+ * careful to dup the actual hash entry if it isn't allocated from
+ * the given pool!
+ */
+ if (add->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);
- /* ### 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;
- for (i = 0; i < add->handlers_remove->nelts; i++) {
- extension_info *exinfo =
- (extension_info*)apr_hash_get(new->extension_mappings,
- 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;
- }
- }
- }
+ if (add->handlers_remove)
+ remove_items(p, add->handlers_remove, new->extension_mappings,
+ (char*)&dummyex.handler - (char*)&dummyex);
new->handlers_remove = NULL;
- if (add->types_remove) {
- suffix = (attrib_info *) add->types_remove->elts;
- for (i = 0; i < add->types_remove->nelts; i++) {
- extension_info *exinfo =
- (extension_info*)apr_hash_get(new->extension_mappings,
- 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;
- }
- }
- }
+ if (add->types_remove)
+ remove_items(p, add->types_remove, new->extension_mappings,
+ (char*)&dummyex.forced_type - (char*)&dummyex);
new->types_remove = NULL;
- if (add->encodings_remove) {
- suffix = (attrib_info *) add->encodings_remove->elts;
- for (i = 0; i < add->encodings_remove->nelts; i++) {
- extension_info *exinfo =
- (extension_info*)apr_hash_get(new->extension_mappings,
- 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;
- }
- }
- }
+ if (add->encodings_remove)
+ remove_items(p, add->encodings_remove, new->extension_mappings,
+ (char*)&dummyex.encoding_type - (char*)&dummyex);
new->encodings_remove = NULL;
new->type = add->type ? add->type : base->type;
@@ -337,14 +316,19 @@
#endif
if (*key == '.')
++key;
- exinfo = (extension_info*)apr_hash_get(m->extension_mappings, key,
- APR_HASH_KEY_STRING);
+ if (!m->extension_mappings) {
+ m->extension_mappings = apr_hash_make(cmd->pool);
+ exinfo = NULL;
+ }
+ else
+ exinfo = (extension_info*)apr_hash_get(m->extension_mappings, key,
+ APR_HASH_KEY_STRING);
if (!exinfo) {
- /* pcalloc sets ->copy to false */
exinfo = apr_pcalloc(cmd->pool, sizeof(extension_info));
key = apr_pstrdup(cmd->pool, key);
apr_hash_set(m->extension_mappings, key,
APR_HASH_KEY_STRING, exinfo);
+ exinfo->pool = cmd->pool;
}
return exinfo;
}
