Re: cvs commit: httpd-2.0/modules/filters mod_include.c

[Cliff Woolley <cl...@yahoo.com>]

On 2 Mar 2001 trawick@apache.org wrote:

> trawick     01/03/02 13:36:23
>
>   Modified:    modules/experimental config.m4 mod_case_filter.c
>                         mod_charset_lite.c mod_disk_cache.c
>                         mod_ext_filter.c
>                modules/filters mod_include.c
>   Log:
>   Use the proper enum for the block/non-block parameter to apr_bucket_read().
>   A couple of these changed in meaning (e.g., 1->APR_BLOCK_READ).
>

There are some related (potential) issues in http_protocol.c that I
noticed a while back but had forgotten about.  Basically, some places in
http_protocol.c call apr_bucket_read() and pass to it their own 'mode'
parameter.  'mode' in the filter sense is of type enum ap_input_mode_t, as
opposed to ap_read_type_e that apr_bucket_read() expects.  It works (for
now), because the two happen to line up in both values and meaning:

typedef enum {
    APR_BLOCK_READ,
    APR_NONBLOCK_READ
} apr_read_type_e;

typedef enum {
    AP_MODE_BLOCKING,
    AP_MODE_NONBLOCKING,
    AP_MODE_PEEK
} ap_input_mode_t;

If one of them ever changed, though, the other one would have to change
too, or we'd silently introduce subtle bugs.  My first reaction would
have been to nix ap_input_mode_t and just use apr_read_type_e... but,
obviously, we can't do that because of AP_MODE_PEEK which has no buckets
equivalent.

We need to do one of three things:

(1) Change the callers in http_protocol.c to do something like this:
       apr_bucket_read(... , (mode == AP_MODE_BLOCKING)
                                 ? APR_BLOCK_READ
                                 : APR_NONBLOCK_READ);

   [ewww, ugly]

(2) Just document that the two have a dependency on one another's values

(3) Give the buckets a PEEK concept and nix ap_input_mode_t.



I really hate to do #1 because it's ugly, but at least it would better
survive changes to one of the enums without corresponding changes to the
other.  I could live with #2, but #3 is interesting and might have
potential uses for other purposes.  Thoughts on that?

--Cliff