apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <cliffwool...@yahoo.com>
Subject file_setaside()
Date Mon, 18 Jun 2001 21:08:49 GMT

I have a few questions about file_setaside.  I'm pasting the function in
here for easy reference.

--------------------------------------------------------------
static apr_status_t file_setaside(apr_bucket *data, apr_pool_t *pool)
{
    apr_bucket_file *a = data->data;
    apr_file_t *fd;
    apr_file_t *f = a->fd;
    apr_pool_t *p = apr_file_pool_get(f);
#if APR_HAS_MMAP
    apr_off_t filelength = data->length;  /* bytes remaining in file past
offset
 */
    apr_off_t fileoffset = data->start;
#endif

    if (apr_pool_is_ancestor(p, pool)) {
        return APR_SUCCESS;
    }

#if APR_HAS_MMAP
    if (file_make_mmap(data, filelength, fileoffset, p)) {
        return APR_SUCCESS;
    }
#endif
    apr_file_dup(&fd, f, p);
    a->fd = fd;
    return APR_SUCCESS;
}
--------------------------------------------------------------

(1) Shouldn't the pool passed into file_make_mmap() and apr_file_dup() be
"pool" and not "p"?  (It'd be easier to see that this is a problem if they
were called "reqpool" and "curpool" instead of "pool" and "p"
respectively, or something like that.)

(2) Why should file_setaside mmap the file?  I'd think that we'd want to
keep it as a file as long as possible to make it easier to use
sendfile()... what am I missing?

(3) You don't really need to dup() the file, do you?  You can palloc a new
apr_file_t in the requested pool and use apr_os_file_get() and
apr_os_file_put() to move the os file handle into it.  mod_file_cache in
Apache does something like this.  It should be cheaper to do this than to
do a full dup(), I think.

Thanks,
--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Mime
View raw message