On Monday 27 September 2010, Jeff Trawick wrote: > On Mon, Sep 27, 2010 at 10:34 AM, <[email protected]> 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
