You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Cliff Woolley <cl...@yahoo.com> on 2001/10/01 20:32:08 UTC
apr_file_close bug?
On Unix, apr_file_close looks like this:
APR_DECLARE(apr_status_t) apr_file_close(apr_file_t *file)
{
apr_status_t rv;
if ((rv = apr_unix_file_cleanup(file)) == APR_SUCCESS) {
apr_pool_cleanup_kill(file->cntxt, file, apr_unix_file_cleanup);
return APR_SUCCESS;
}
return rv;
}
So if file_cleanup fails, then we won't kill the cleanup, and when we
cleanup the file->cntxt pool, we'll run the cleanup again. We used to
have this exact same behavior in the directory cleanups and it caused
segfaults.
I think it should just say this:
{
apr_pool_cleanup_kill(file->cntxt, file, apr_unix_file_cleanup);
return apr_unix_file_cleanup(file);
}
Which is exactly equivalent to what apr_dir_close() does now that it's
been fixed. Thoughts?
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: apr_file_close bug?
Posted by Ryan Bloom <rb...@covalent.net>.
On Monday 01 October 2001 11:32 am, Cliff Woolley wrote:
> On Unix, apr_file_close looks like this:
>
> APR_DECLARE(apr_status_t) apr_file_close(apr_file_t *file)
> {
> apr_status_t rv;
>
> if ((rv = apr_unix_file_cleanup(file)) == APR_SUCCESS) {
> apr_pool_cleanup_kill(file->cntxt, file, apr_unix_file_cleanup);
> return APR_SUCCESS;
> }
> return rv;
> }
>
>
> So if file_cleanup fails, then we won't kill the cleanup, and when we
> cleanup the file->cntxt pool, we'll run the cleanup again. We used to
> have this exact same behavior in the directory cleanups and it caused
> segfaults.
>
> I think it should just say this:
>
> {
> apr_pool_cleanup_kill(file->cntxt, file, apr_unix_file_cleanup);
> return apr_unix_file_cleanup(file);
> }
>
> Which is exactly equivalent to what apr_dir_close() does now that it's
> been fixed. Thoughts?
++1! In fact, all of the cleanups should do this. I didn't know about
apr_pool_cleanup_kill when I wrote all of those functions.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: apr_file_close bug?
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Oct 01, 2001 at 01:36:18PM -0500, William A. Rowe, Jr. wrote:
> From: "Cliff Woolley" <cl...@yahoo.com>
> Sent: Monday, October 01, 2001 1:32 PM
>
>
> > I think it should just say this:
> >
> > {
> > apr_pool_cleanup_kill(file->cntxt, file, apr_unix_file_cleanup);
> > return apr_unix_file_cleanup(file);
> > }
> >
> > Which is exactly equivalent to what apr_dir_close() does now that it's
> > been fixed. Thoughts?
>
> Thinking ... yes.
>
> Clear, concise, and less redundant.
Umm... no. Use apr_pool_cleanup_run(). That will run *and* remove the
cleanup in one fell swoop. So your code is even simpler:
return apr_pool_cleanup_run(file->cntxt, file, apr_unix_file_cleanup);
You could even see this as an "atomic" run-remove call.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: apr_file_close bug?
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Cliff Woolley" <cl...@yahoo.com>
Sent: Monday, October 01, 2001 1:32 PM
> I think it should just say this:
>
> {
> apr_pool_cleanup_kill(file->cntxt, file, apr_unix_file_cleanup);
> return apr_unix_file_cleanup(file);
> }
>
> Which is exactly equivalent to what apr_dir_close() does now that it's
> been fixed. Thoughts?
Thinking ... yes.
Clear, concise, and less redundant.