Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <0a3b01c1a43e$630fc4f0$94c0b0d0@v505>
From: "William A. Rowe, Jr." <wrowe@covalent.net>
To: "Justin Erenkrantz" <jerenkrantz@ebuilt.com>
Cc: <dev@apr.apache.org>
References: <097701c1a430$49af8670$94c0b0d0@v505>
 <20020123174417.GZ28051@ebuilt.com> <09e601c1a439$c6ad1e90$94c0b0d0@v505>
 <20020123182616.GC28051@ebuilt.com>
Subject: Re: bug in apr_brigade.c
Date: Wed, 23 Jan 2002 12:45:56 -0600
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

From: "Justin Erenkrantz" <jerenkrantz@ebuilt.com>
Sent: Wednesday, January 23, 2002 12:26 PM


> On Wed, Jan 23, 2002 at 12:07:33PM -0600, William A. Rowe, Jr. wrote:
>
> > We've been through this; read the threads please.  But long story short, a
> > 'bucket' is a unit that _may_ be read into memory (although it need not)
> > while the 'brigade' may obviously be huge.
> > 
> > The alternative, buckets > size_t is such a massive rewrite you don't
> > want to contemplate it ;)
> 
> This type confusion just seems horrific.  I don't buy into this
> distinction at all.  An apr_off_t represents an offset into a file.
> apr_size_t should be the size of the bucket/brigade.  If we want
> to do it correctly, I'd say that brigade lengths should be 64-bit
> ints (apr_int64_t).

Oh woe is me.  types are confusing.  pity.

Guess what, we had some 70 or so type errors when I started to cleanly
differentiate buckets and brigades.  SEVENTY friggin errors!  Because
noone paid it attention.  Let people just cast around it, why not.
Segfaults on the first 5GB bucket?  Oh well.

I have one comprimize.  Brigades get clamped BACK DOWN to apr_size_t.
No further discussion.  If it's bigger than memory, APR apps doen't 
care to deal with it.  

I had two choices.  Make buckets handle huge data, or brigades handle
smaller buckets.  The choice was obvious by the overhead of code required,
and extra testing, when all we wanted to do was READ A BUCKET INTO MEMORY :-)

Handling huge objects in a single bucket is unwieldly.  For example, 
your next point...


> > Win32 for one, right now.  A number of unicies, if anyone ever gets it together
> > and actually hacks this out right.  I assume OS2 (since 64bit file pointers go
> > back to the pre-divorce days) but they may not be implemented.  Dunno about NW.
> 
> Unlikely Unix platforms will ever get largefile support (I coded up
> the autoconf magic and is listed in APR's STATUS).  Some of them 
> (Linux, in particular) disable sendfile() with 64-bit file support.
> I didn't have the heart to follow through on this.  If someone wants
> to pick it up...

This is exactly my point.  sendfile is restricted mostly to ssize_t transmissions, 
across Unicies.  sendfile is used to send a single file bucket.  GUESS WHAT?
My patch fixed those buckets to ssize_t.  So there is no problem, sendfile
works right now with bucket brigades on LARGEFILE platforms.


And on the altogether different topic :)

> > Buckets can always contain NULLs.  Code that assumes otherwise will be bitten
> > someday [yet another bugtraq expose'.]
> 
> Oh, you mean embedded NULLs.  Perhaps, but I think that's outside of
> the scope of this function.  I'll change the prototype to return a
> length.  Let's get the functions right before anyone actually uses
> them.  =)  -- justin

I rather suspected which is why I didn't gripe louder.  Perhaps it should
have a _str in the fn name, if that is all it's capable of.

My point about patching apr_file_dup2 before we release.  We just added it
in .31-dev, why not get it right before tagging :-?