Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 1 Apr 2002 23:10:50 -0500 (EST)
From: Cliff Woolley <jwoolley@virginia.edu>
To: Bill Stoddard <bill@wstoddard.com>
cc: APR Development List <dev@apr.apache.org>
Subject: Re: apr_bucket_file_create() args...
In-Reply-To: <021901c1d9f9$52035a00$05000100@sashimi>
Message-ID: 
 <Pine.LNX.4.44.0204012304350.23932-100000@deepthought.cs.virginia.edu>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII

On Mon, 1 Apr 2002, Bill Stoddard wrote:

> This looks broken to me (at least on Windows). apr_off_t is an int64 and
> apr_size_t is an int.
>
> APU_DECLARE(apr_bucket *) apr_bucket_file_create(apr_file_t *fd,
>                                                  apr_off_t offset,
>                                                  apr_size_t len, apr_pool_t *p,
>                                                  apr_bucket_alloc_t *list)
>
> I think both the len and offset should be apr_off_t.


This has been a big point of contention.  I agree that it's very awkward,
but I don't currently know of a better way to do it.  The problem is that
the current rules state that no one bucket can contain more than an
apr_size_t's worth of data.  So if a file is bigger than an apr_size_t,
the caller creating the bucket must in fact create multiple buckets and
string them together.  So we get lots of code like this (this example is
from the default_handler() in Apache's core.c:

--------------------------------------------------------------
#if APR_HAS_LARGE_FILES
    if (r->finfo.size > AP_MAX_SENDFILE) {
        /* APR_HAS_LARGE_FILES issue; must split into mutiple buckets,
         * no greater than MAX(apr_size_t), and more granular than that
         * in case the brigade code/filters attempt to read it directly.
         */
        apr_off_t fsize = r->finfo.size;
        e = apr_bucket_file_create(fd, 0, AP_MAX_SENDFILE, r->pool,
                                   c->bucket_alloc);
        while (fsize > AP_MAX_SENDFILE) {
            apr_bucket *ce;
            apr_bucket_copy(e, &ce);
            APR_BRIGADE_INSERT_TAIL(bb, ce);
            e->start += AP_MAX_SENDFILE;
            fsize -= AP_MAX_SENDFILE;
        }
        e->length = (apr_size_t)fsize; /* Resize just the last bucket */
    }
    else
#endif
        e = apr_bucket_file_create(fd, 0, (apr_size_t)r->finfo.size,
                                   r->pool, c->bucket_alloc);

    APR_BRIGADE_INSERT_TAIL(bb, e);
--------------------------------------------------------------

I personally have always thought this was bogus.  But how else can you do
it and still maintain the rule that a bucket can be at most an apr_size_t?
The bucket create function would have to contain the logic above, but then
that would mean that a bucket create function could create a string of
buckets rather than just a single bucket, which currently none of our
bucket create functions are allowed to do.  Maybe we should just create a
utility function for this.  Or perhaps the decision to have buckets be at
most an apr_size_t should be reversed.  But word on the street is that
that causes problems elsewhere.  I haven't personally had time to look
into it, but I can imagine that it could get pretty ugly in some places...

--Cliff


--------------------------------------------------------------

   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA