You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@gmail.com> on 2010/09/27 16:58:53 UTC
Re: svn commit: r1001757 - /httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
On Mon, Sep 27, 2010 at 10:34 AM, <sf...@apache.org> wrote:
> Author: sf
> Date: Mon Sep 27 14:34:29 2010
> New Revision: 1001757
>
> URL: http://svn.apache.org/viewvc?rev=1001757&view=rev
> Log:
> fix another null pointer dereference found by clang
>
> Modified:
> httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
>
> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c?rev=1001757&r1=1001756&r2=1001757&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c Mon Sep 27 14:34:29
> 2010
> @@ -255,9 +255,11 @@ char *ssl_var_lookup(apr_pool_t *p, serv
> }
> /* all other env-variables from the parent Apache process */
> else if (strlen(var) > 4 && strcEQn(var, "ENV:", 4)) {
> - result = apr_table_get(r->notes, var+4);
> - if (result == NULL)
> - result = apr_table_get(r->subprocess_env, var+4);
> + if (r != NULL) {
> + result = apr_table_get(r->notes, var+4);
> + if (result == NULL)
> + result = apr_table_get(r->subprocess_env, var+4);
> + }
> if (result == NULL)
> result = getenv(var+4);
> }
>
>
I guess you decided against splitting ENV: handling between the request_rec
section earlier and the non-request_rec/conn_rec section, which doesn't
really make sense for this.
default:
...
else if (strlen(var) > 4 && strcEQn(var, "ENV:", 4)) {
result = apr_table_get(r->notes, var+4);
if (result == NULL)
result = apr_table_get(r->subprocess_env, var+4);
}
}
That would leave the following logic in the "Totally independent stuff"
section:
if (result == NULL) {
...
else if (strlen(var) > 4 && strcEQn(var, "ENV:", 4)) {
result = getenv(var+4);
}
}
Big shrug... I guess it is just the comments which are busted. (The
mapping of "ENV:foo" to r->request_notes is surprising; I haven't seen that
before.)
Re: svn commit: r1001757 - /httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Monday 27 September 2010, Jeff Trawick wrote:
> On Mon, Sep 27, 2010 at 10:34 AM, <sf...@apache.org> wrote:
> > Author: sf
> > Date: Mon Sep 27 14:34:29 2010
> > New Revision: 1001757
> >
> > URL: http://svn.apache.org/viewvc?rev=1001757&view=rev
> > Log:
> > fix another null pointer dereference found by clang
> >
> > Modified:
> > httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
> >
> > Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_en
> > gine_vars.c?rev=1001757&r1=1001756&r2=1001757&view=diff
> >
> > =================================================================
> > ============= --- httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
> > (original) +++ httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
> > Mon Sep 27 14:34:29 2010
> > @@ -255,9 +255,11 @@ char *ssl_var_lookup(apr_pool_t *p, serv
> >
> > }
> > /* all other env-variables from the parent Apache process
> > */ else if (strlen(var) > 4 && strcEQn(var, "ENV:", 4))
> > {
> >
> > - result = apr_table_get(r->notes, var+4);
> > - if (result == NULL)
> > - result = apr_table_get(r->subprocess_env,
> > var+4); + if (r != NULL) {
> > + result = apr_table_get(r->notes, var+4);
> > + if (result == NULL)
> > + result = apr_table_get(r->subprocess_env,
> > var+4); + }
> >
> > if (result == NULL)
> >
> > result = getenv(var+4);
> >
> > }
>
> I guess you decided against splitting ENV: handling between the
> request_rec section earlier and the non-request_rec/conn_rec
> section, which doesn't really make sense for this.
>
> default:
> ...
> else if (strlen(var) > 4 && strcEQn(var, "ENV:", 4)) {
> result = apr_table_get(r->notes, var+4);
> if (result == NULL)
> result = apr_table_get(r->subprocess_env,
> var+4); }
> }
>
> That would leave the following logic in the "Totally independent
> stuff" section:
>
> if (result == NULL) {
>
> ...
>
> else if (strlen(var) > 4 && strcEQn(var, "ENV:", 4)) {
> result = getenv(var+4);
> }
> }
>
> Big shrug... I guess it is just the comments which are busted.
> (The mapping of "ENV:foo" to r->request_notes is surprising; I
> haven't seen that before.)
Actually, I haven't considered this. I guess that splitting it may be
clearer and makes the code fit the comments. Changed in r1001795