httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject Re: cvs commit: apache-2.0/src/main http_protocol.c
Date Thu, 09 Nov 2000 18:05:11 GMT

> You could look at it like the copy function copies the bucket including the data
> unless there is a possible optimization for that bucket type in that the data can be
> doubly referenced.  It seems to me that this is logical to place in the hands of the
> bucket code (not the caller) because the bucket code already knows what kind of
> bucket it is; the caller (as Greg pointed out) would have loads of headaches if some
> buckets could be copied in one way, other buckets could be changed and then copied,
> etc.  Lots of room for bugs there.

Allow me to boil this down a little bit.  The design that you and Greg are
proposing basically says one thing.  "Some buckets can be on multiple
brigades, others can't."  If pipe and socket buckets copy the data when
they are dup'ed, then it will always be impossible for them to be put on
more than one brigade.  As for bugs, we are talking about this code:

    ap_bucket *e;
    ap_bucket *f;
    if (e->length == -1) {
    f = ap_bucket_dup(e,...)

I'm not overly concerned that this is too complex.  If this is the action
that you want, then I would argue that you really want two functions.  One
that is a bucket function, dup, which duplicates a bucket, but leaves
anything in that bucket alone.  The second is a wrapper around the dup
function which may read if the bucket->length == -1.

> I'd argue that none of the bucket functions should really be returning APR_ENOTIMPL.
>  In other words, it should only be safe to do nothing (where APR_ENOTIMPL is
> currently returned) when the caller can safely assume that if something needed to
> happen, then it did, and if it didn't need to happen, then it can ignore that fact. 
> In other words, the APR_ENOTIMPL return values for these functions should arguably
> be APR_SUCCESS.  For example, it's safe for the caller to ignore the return value
> from a destroy function that doesn't do anything because the bucket cannot be
> destroyed (eg, immortal buckets).  But my understanding of APR_ENOTIMPL is "oops,
> you can'd do that, you'd better do something else".  But maybe my understanding of
> the semantics of those return values is off... all of that is just a side issue.

That really isn't the meaning of APR_ENOTIMPL.  The meaning of
APR_ENOTIMPL is we didn't implement this function, either because it
didn't make sense, or it wasn't possible, or we didn't have time, etc.  It
says nothing about success or failure.  If a function wasn't implemented,
that is useful information for the programmer to know, it most definately
is not a success condition.

> My point is that it *is* possible to split() and copy() all of these bucket types. 
> The fact that the "color" of the bucket changes in the background is irrelevant...
> the caller shouldn't care.  It puts in one bucket, it gets two out.  That's all it

My problem with this, is that if you define dup and split differently for
pipe/socket buckets it isn't "one goes in, two come out".  If pipe/socket
buckets read then dup/split, then what is really happening is "one goes
in, three come out".  Count the buckets.  Think of it this way.  What
happens if you aren't currently in a brigade when you dup or split a
bucket.  With all regular buckets, you get two distinct buckets
out.  Those buckets can be inserted into any briade easily, because you
know that you have exactly two separate and distinct buckets.  Now, split
or dup a pipe/socket bucket.  What you get back isn't two separate
buckets.  What you get back is one separate bucket and one list of two
buckets.  That is much more likely to cause bugs than the if statement

>  Okay, splitting an EOS bucket is questionable, but for any bucket type that really
> represents DATA, the above holds true.

Just for completeness, FLUSH is just like EOS, so thats two
exceptions.  :-)


Ryan Bloom               
406 29th St.
San Francisco, CA 94131

View raw message