apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <jwool...@virginia.edu>
Subject Re: apr_bucket_file_create() args...
Date Tue, 02 Apr 2002 04:10:50 GMT
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 (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,
        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 */
        e = apr_bucket_file_create(fd, 0, (apr_size_t)r->finfo.size,
                                   r->pool, c->bucket_alloc);


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 Woolley
   Charlottesville, VA

View raw message