[PATCH multi-env] remove size() functions, empty string on n==0, apreq_fwritev() rewrite

[Max Kellermann <ma...@duempel.org>]

04-remove_size_functions.patch
- remove apreq_param_size() and apreq_cookie_size()

05-remove_empty_string_on_n_eq_0.patch
- make apreq_join() and apreq_params_as_string() return an empty
  string on n==0

06-rewrite_fwritev.patch
- rewrite apreq_fwritev()
- no recursion; while(1) instead.
- optimization: eliminate one loop checking the whole iovec array
- check for endless loop (len == 0)

please review the last patch very carefully, because I havn't tested
that (writing a test for apreq_brigade_fwrite() is on my TODO list).

Another remark: error.c checks the apr_strerror() value for
APR_EGENERAL. A message for that was not implemented until httpd 2.1,
i.e. the test fails with httpd 2.0.53.

Max

Re: [PATCH multi-env] remove size() functions, empty string on n==0, apreq_fwritev() rewrite

[Joe Schaefer <jo...@sunstarsys.com>]

Max Kellermann <ma...@duempel.org> writes:

> 04-remove_size_functions.patch
> - remove apreq_param_size() and apreq_cookie_size()
>
> 05-remove_empty_string_on_n_eq_0.patch
> - make apreq_join() and apreq_params_as_string() return an empty
>   string on n==0
>
> 06-rewrite_fwritev.patch
> - rewrite apreq_fwritev()
> - no recursion; while(1) instead.
> - optimization: eliminate one loop checking the whole iovec array
> - check for endless loop (len == 0)

Thanks Max, applied!

-- 
Joe Schaefer

Re: [PATCH multi-env] remove size() functions, empty string on n==0, apreq_fwritev() rewrite

[Joe Schaefer <jo...@sunstarsys.com>]

Max Kellermann <ma...@duempel.org> writes:

> On 2005/03/11 04:03, Joe Schaefer <jo...@sunstarsys.com> wrote:

[...]

>> Ugh, that's bad.  How about we replace all APR_EGENERAL error
>> codes with APREQ_ERROR_GENERAL?
>
> What's the difference between the two?
>
> Disadvantage of custom status codes is that ap_log_rerror() can't
> handle a custom libapreq status code correctly...

That's infinitely better than reusing the status codes from apr.
If it's an apreq function that is choking, the status code should
be in the APREQ_ERROR_ range.  We'll have to deal with ap_log_rerror 
not understanding those errors anyway, so by adjusting all the
APR_EGENERAL uses to APREQ_ERROR_GENERAL, the required wrapper code 
for ap_log_rerror will automically DTRT.

> or when some other function is just passing a status code from
> libapreq to its caller, which then handles it as "normal" APR status
> code.

Not our problem.  In an ideal world, all possible status codes
(or ranges of status codes) that a function may return are 
documented.  If a function wants to hide the fact that they
are using apreq, they need to translate the APREQ_ERROR
codes into something else before exposing it to the user.


> So, is APREQ_ERROR_GENERAL really needed?

Needed, no. Better than APR_EGENERAL?  I think so.

-- 
Joe Schaefer

Re: [PATCH multi-env] remove size() functions, empty string on n==0, apreq_fwritev() rewrite

[Max Kellermann <ma...@duempel.org>]

On 2005/03/11 04:03, Joe Schaefer <jo...@sunstarsys.com> wrote:
> > Another remark: error.c checks the apr_strerror() value for
> > APR_EGENERAL. A message for that was not implemented until
> > httpd 2.1, i.e. the test fails with httpd 2.0.53.
> 
> Ugh, that's bad.  How about we replace all APR_EGENERAL error
> codes with APREQ_ERROR_GENERAL?

What's the difference between the two?

Disadvantage of custom status codes is that ap_log_rerror() can't
handle a custom libapreq status code correctly... or when some other
function is just passing a status code from libapreq to its caller,
which then handles it as "normal" APR status code.

So, is APREQ_ERROR_GENERAL really needed?

Max

Re: [PATCH multi-env] remove size() functions, empty string on n==0, apreq_fwritev() rewrite

[Joe Schaefer <jo...@sunstarsys.com>]

Max Kellermann <ma...@duempel.org> writes:

> please review the last patch very carefully, because
> I havn't tested that

It looks ok to me after a quick scan.  I'll study it carefully
before committing it though.

> Another remark: error.c checks the apr_strerror() value for
> APR_EGENERAL. A message for that was not implemented until
> httpd 2.1, i.e. the test fails with httpd 2.0.53.

Ugh, that's bad.  How about we replace all APR_EGENERAL error
codes with APREQ_ERROR_GENERAL?

-- 
Joe Schaefer