apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@covalent.net>
Subject Re: bug in apr_brigade.c
Date Wed, 23 Jan 2002 18:07:33 GMT
From: "Justin Erenkrantz" <jerenkrantz@ebuilt.com>
Sent: Wednesday, January 23, 2002 11:44 AM

> On Wed, Jan 23, 2002 at 11:04:44AM -0600, William A. Rowe, Jr. wrote:
> > This code below is buggy on LARGEFILE platforms, since the apr_off_t tmp
> > brigade length may not fit in memory (the apr_size_t actual.)
> Ah, yes, my bad.  I forgot that brigades are apr_off_t-based and
> buckets are apr_size_t based.  How about making buckets apr_off_t
> based instead?  What's the rationale behind having buckets being
> apr_size_t?

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 ;)

> (BTW, what LARGEFILE platforms are there?)

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.

> > This is actually really crufty - since this code needs to deal with -1
> > length buckets - they are read [seperately] to resolve the length and 
> > then recopied into the flatten bucket [from zero copy to three copy 
> > in 10e-6 seconds].  Actually that's an assumption that apr_brigade_length
> > still does so, I'm not certain.  If not, I presume apr_brigade_length
> > may have returned -1.
> Nah, it doesn't need to deal with -1 length buckets.  
> apr_brigade_length with "read_all" set to 1 will exhaust all buckets
> to get the true length.

So it's simply dirty, not broken :)

> > And I'm really uncomfortable about 'adding' null terminators.  But that's
> > a different point.
> Do you want to return a size instead?  We could do that.  -- justin

Buckets can always contain NULLs.  Code that assumes otherwise will be bitten
someday [yet another bugtraq expose'.]


View raw message