Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
From: Cliff Woolley <cliffwoolley@yahoo.com>
To: new-httpd@apache.org
MMDF-Warning: Parse error in original version of preceding line at
 mail.virginia.edu
Subject: RE: socket_read?
Date: Sat, 17 Feb 2001 23:28:20 -0500
Message-ID: <NEBBKBPPOLLGMBILHACFMEEHCBAA.cliffwoolley@yahoo.com>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
In-Reply-To: <3A8F2C88.9B6DE56C@algroup.co.uk>
Importance: Normal

> -----Original Message-----
> >     if ((rv=apr_bucket_read(e,&s,&len,APR_BLOCK_READ)) != APR_SUCCESS)
> >         return rv;

> Break in what sense? It will cause them to return EOF when they read an
> EOF. Is that broken? If you carry on reading past an EOF what is the
> expected behaviour from the underlying filter? Surely once its returned
> EOF its game over?

Nope--at least not always.  This particular snippet is from
apr_brigade_partition().  That function loops through the entire brigade one
bucket at a time, trying to ensure that there's a bucket boundary at a given
byte offset into the brigade.  It avoids reading buckets as best it can, but
this can't be avoided for pipe and socket buckets, which have an indeterminate
length before being read.  APR_EOF within apr_bucket_read() of a pipe or socket
bucket just means "you've read all there is to read, this bucket is now empty."
But that's a success case, at least as far as apr_brigade_partition is
concerned.  Consider a pipe bucket in the middle of a brigade being passed down
the chain from a CGI or something similar.  There very well might be buckets
after the pipe bucket.  Just because the pipe bucket's empty, that doesn't mean
we abort from apr_brigade_partition, it just means we ignore it and move on to
the next bucket.

> Of course, the right answer may be that socket_read shouldn't return EOF
> at all, it should return an EOS bucket.

Nooooo... definitely not.  You'd easily end up with multiple EOS buckets in the
brigade if socket_read() did that, which is very bad (plus you'd break the above
even worse).  As Jeff pointed out, an EOF condition in a particular bucket may
or may not represent the end of the entire brigade (which is what an EOS bucket
is for).

> But that's a different matter -
> what I'm currently trying to establish is that it should do the same
> thing regardless of whether it is blocking or not.

That might be.  I'd personally argue that they're both success cases... you've
read as much as there is to read from this bucket.  Move on to the next bucket.

> I actually suspect it is doing the wrong thing in both cases currently
> (i.e. empty bucket in one,

Yep.  In this case, the pipe/socket bucket should just remove itself from the
brigade, to avoid trying to re-read it on another pass through the brigade.
That's easy enough to fix.

> APR_EOF in the other, instead of EOS bucket
> in both), so once we've all agreed the first point, let's figure this
> one out, too!

We could make both of them return APR_SUCCESS, which makes sense for
type-agnostic callers that just want to know nothing catastrophic happened and
that they can move on to the next bucket in the brigade.

We could return APR_EOF for both, that might make sense for callers that know
about different kinds of bucket.  But I just don't see how it does the caller
any good... the EOS bucket at the end of the brigade should be all that's cared
about, right?  What am I missing?  I'm going to go look more closely at mod_tls
to see if this makes more sense to me...

Bottom line: I don't have a problem with making them both return the same value,
whatever value that is... just no EOS buckets, please.  =-)


PS: Jon Travis is the one that originally submitted the patch to make it return
APR_EOF in one case and APR_SUCCESS in the other... maybe he'd care to comment
on this?

> BTW, I'm about to check in a working mod_tls. Yay!

Cool.  =-]