httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject Re: svn commit: r1001757 - /httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
Date Mon, 27 Sep 2010 16:08:05 GMT
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

Mime
View raw message