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: PATCH [multi-env]: inline functions, apreq_initialize()
Date Mon, 07 Feb 2005 19:46:57 GMT
Max Kellermann <max@duempel.org> writes:

> 12-yet_more_inline_functions.patch
> - also convert the apreq_cookie_t and apreq_param_t helper macros to
>   inline functions
> - repaired apreq_param_brigade which wouldn't compile (another
>   advantage of inline functions - with macros, this wasn't even
>   noticed)


I think we should just remove apreq_param_brigade.  The
general issue with exposing a bucket brigade is that
there are side effects which happen during file-bucket reads.
Users that want to read  data from the brigade should
make a copy of the brigade first, and then successively
read+delete the buckets.

Knowledge about this issue is beneficial beyond just libapreq2,
so documenting it is probably a better idea than just trying to 
encapsulate it in some apreq_param_foo function.

> things to be discussed:
> - rename apreq.h to apreq_util.h? I find it confusing to include
>   apreq.h and not to get any of the important libapreq2
>   features. Maybe rename "apreq_env.h" to "apreq.h" then, because it
>   provides the core features?


+1.  Unless someone objects, I'll take care of this rename in a few
days, once we're caught up on everything else.

> - it's standard to make the target "check" run the test routines; in
>   libapreq2, "check" only builds the test programs and "test" runs
>   them
>
> - target "all" builds shared libraries only used by test programs, but
>   does not build the test program itself; fix that?

I don't mind making "check" and "test" synonyms (perl guys prefer
"test").  The ambitious plan for the t/ would be to remove CuTest.*
and replace it with an Apache::Test-based C framework, modeled 
around independent C executables that emit TAP (the "ok/fail" 
protocol).  We should add an svn:external to the /asf/perl/Apache-Test 
repos and include that in our releases, just like modperl does.
That way we can remove the Apache::Test dependency from our 
prereqs.

> - I still need to write code to test the subrequest handling in
>   mod_apreq.

Addressing subrequests in the tests would definitely
improve mod_apreq.

> - move each parser into its own .c file?

A good idea; but there's some private interaction between the 
header parser and the mfd parser that I think we'd need to
look closer at.

> - rename apreq_env_handle_t to apreq_t?

I think it should be apreq_handle_t, to complement apreq_module_t.

-- 
Joe Schaefer


Mime
View raw message