httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Raymond S Brand <r...@rsbx.net>
Subject Re: cvs commit: apache-1.3/src/modules/standard mod_autoindex.c mod_include.c
Date Fri, 14 May 1999 18:14:22 GMT
Dean Gaudet wrote:
> 
> On Thu, 13 May 1999, Raymond S Brand wrote:
> 
> > Dean Gaudet wrote:
> > >
> > > ap_pool_join is an advisory debugging tool -- alloc.c does nothing to
> > > ensure that you preserve the lifetimes appropriately.
> >
> > I understand that. Please apply  the patch to mod_include and look at
> > where the ap_make_sub_pool() I added is called.
> 
> Yeah I saw that:
> 
>         /* Kludge --- Doing this allows the caller to safely destroy the
>          * sub_req
>          */
>         r->pool = ap_make_sub_pool(r->pool);
> 
> Sorry, but that's just way off my bogosity meter.

No more bogus than the current situation in mod_include. That code path
is only executed for sub_reqs created by mod_include (works fine, they're
about to be destroyed) or for sub_reqs created in another module and flagged
that they should be treated as a sub_req created in mod_inlcude (again
works fine, they're also about to be destroyed).

> Consider this sequence:
> 
>     in module foo: my_p = ap_make_sub_pool(r->pool);
>                    my_table = ap_make_table(my_p, 5);
>     in your hacked mod_include: r->pool = ap_make_sub_pool(r->pool);
>     in module foo: x = ap_pstrdup(r->pool, blahblahblah);
>                    ap_table_setn(my_table, "abc", x);
> 
> That's broken.  And module foo isn't what's broken, it's doing something
> that fits within the ancestor relationship... only the ancestry has been
> broken.  Although I think the damage really only becomes apparent in
> a cleanup.

I fail to see how the above call sequence can happen. Can you flesh it out
some? And are there any ap_*sub_req*() calls in the sequence?

> Let's just say I'm going to take a lot more convincing that futzing with
> r->pool is fine.  In general I consider that to be a read-only field.
> You'd certainly screw up if we had contexts in the pool like we're
> talking about for 2.0.

It may not survive with contexts in 2.0, but it's not clear that the existing
mod_include can coexist with contexts in 2.0 either.

The sub_pool business is ONLY there so that a sub_req can be destroyed. I'm
tired of this argument. Use the following;

In mod_include:
@@ -2376,6 +2365,38 @@
         return OK;
     }
 
+#define SUB_REQ_STRING "Sub request to mod_include"
+#define PARENT_STRING  "Parent request to mod_include"
+
+    if (ap_table_get(r->notes, SUB_REQ_STRING)) {
+       request_rec *p = r->main;
+       /* The note is a flag to mod_include that this request is actually
+        * a subrequest from another module and that mod_include needs to
+        * treat it as if it's a subrequest from mod_include.
+        *
+        * HACK ALERT!
+        * There is no good way to pass the parent request_rec to mod_include.
+        * Tables only take string values and there is nowhere appropriate in
+        * in the request_rec that can safely be used.
+        *
+        * So we search up the chain of requests and redirects looking for
+        * the parent request.
+        */
+       while (p) {
+           if (ap_table_get(p->notes, PARENT_STRING)) {
+               /* Kludge --- See below */
+               ap_set_module_config(r->request_config, &includes_module, p);
+
+               ap_add_common_vars(p);
+               ap_add_cgi_vars(p);
+               add_include_vars(p, DEFAULT_TIME_FORMAT);
+               ap_table_unset(r->notes, SUB_REQ_STRING);
+               break;
+           }
+           p = (p->prev) ? p->prev : p->main;
+       }
+    }
+
     if ((parent = ap_get_module_config(r->request_config, &includes_module))) {
        /* Kludge --- for nested includes, we want to keep the subprocess
         * environment of the base document (for compatibility); that means


In mod_autoindex:
@@ -924,6 +924,9 @@
      ap_rputs("</PRE>\n", r);
  }
  
+ /* See mod_include */
+ #define SUB_REQ_STRING  "Sub request to mod_include"
+ #define PARENT_STRING   "Parent request to mod_include"
  
  /*
   * Handle the preamble through the H1 tag line, inclusive.  Locate
@@ -969,6 +972,8 @@
                  if (! suppress_amble) {
                      emit_preamble(r, title);
                  }
+                 ap_table_add(r->notes, PARENT_STRING, "");
+                 ap_table_add(rr->notes, SUB_REQ_STRING, "");
                  /*
                   * If there's a problem running the subrequest, display the
                   * preamble if we didn't do it before -- the header file
@@ -1006,9 +1011,6 @@
      if (emit_H1) {
          ap_rvputs(r, "<H1>Index of ", title, "</H1>\n", NULL);
      }
-     if (rr != NULL) {
-         ap_destroy_sub_req(rr);
-     }
  }
  
  
@@ -1045,6 +1047,8
           */
          if (rr->content_type != NULL) {
             if (!strcasecmp("text/html", rr->content_type)) {
+                 ap_table_add(r->notes, PARENT_STRING, "");
+                 ap_table_add(rr->notes, SUB_REQ_STRING, "");
                  if (ap_run_sub_req(rr) == OK) {
                      /* worked... */
                      suppress_sig = 1;
@@ -1072,9 +1076,6 @@
      if (!suppress_post) {
          ap_rputs("</BODY></HTML>\n", r);
      }
-     if (rr != NULL) {
-         ap_destroy_sub_req(rr);
      }
  }
  
  


Raymond S Brand

Mime
View raw message