httpd-apreq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Schaefer <joe+gm...@sunstarsys.com>
Subject Re: Bug in apreq_attr_to_type macro
Date Thu, 18 Sep 2003 02:16:49 GMT
Bojan Smojver <bojan@rexursive.com> writes:

> The following macro produces unwanted results if P == NULL.
> 
> #define apreq_attr_to_type(T,A,P) ( (T*) ((char*)(P)-offsetof(T,A)) )
> 
> Basically, you get back a non-NULL value, thus believing that this is
> an actual valid memory address. Two problems with that: you can't tell
> that you actually found nothing and the code segfaults. Something like
> this would be a bit better IMHO:
> 
> #define apreq_attr_to_type(T,A,P) \
>         ( (P) ? ((T*) ((char*)(P)-offsetof(T,A))) : NULL )
> 
> I've tested this in my code and it seems to work, at least for
> apreq_cookie() call, which relies on the above macro (through
> apreq_value_to_cookie macro) to fetch cookies from the table. I've
> tested this with Apache 2.0.47 on Red Hat 9. 

Thanks! I have a different take on the situation, though.
IMO this represents a bug in apreq_cookie()- we *really should* 
check the return value of apr_table_get before feeding it to the 
macro.

Since apreq_attr_to_type is used a lot, and most of the calls are
successive, I'd also prefer not adding the conditional to its
definition.  If we did, more often than not we'd wind up doing two
"non-NULL" tests instead of just one, and I'm also not sure how well C
compilers will be able to optimize those expressions, i.e.

  cookie = apreq_value_to_cookie(apreq_char_to_value(char_ptr));

With the current definition of apreq_attr_to_type, a decent compiler 
should be able to reduce the right side to a single pointer subtraction.
Of course I'm guessing here- maybe someone could check the output of
a few common compilers on such expressions?

-- 
Joe Schaefer


Mime
View raw message