Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 82509 invoked by uid 500); 18 Jun 2001 21:31:21 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 82480 invoked from network); 18 Jun 2001 21:31:16 -0000 Date: Mon, 18 Jun 2001 14:33:24 -0700 (PDT) From: X-Sender: To: Cliff Woolley Cc: Subject: Re: file_setaside() In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N > 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.) I think this is a bug, but I need to look closer...... Yep, bug. > (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? We are going to be copying something. I figured mmap'ing the file would be a bit better, because we could write the file out. Either way, it doesn't really matter. > (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. The file was opened with the request->pool. If we just apr_os_file_get/put, we will still close the file when the request_pool is cleared. We have to dup, or the file won't be available to us, and the original bug will be back. Ryan _______________________________________________________________________________ Ryan Bloom rbb@apache.org 406 29th St. San Francisco, CA 94131 -------------------------------------------------------------------------------