You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ruediger Pluem <rp...@apache.org> on 2021/09/10 06:50:01 UTC
Re: svn commit: r1893204 - in /apr/apr/trunk/file_io: os2/filedup.c
unix/filedup.c win32/filedup.c
On 9/10/21 2:51 AM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Fri Sep 10 00:51:53 2021
> New Revision: 1893204
>
> URL: http://svn.apache.org/viewvc?rev=1893204&view=rev
> Log:
> apr_file_setaside: don't blindly kill the old cleanup and file descriptor.
>
> There is no cleanup with APR_FOPEN_NOCLEANUP, so apr_pool_cleanup_kill() can
> go in the !(old_file->flags & APR_FOPEN_NOCLEANUP) block.
>
> The file descriptor can't be invalidated either, the file may be split in
> multiple buckets and setting aside one shouldn't invalidate the others.
Hmm. Is it safe to use the old_file any longer after apr_file_setaside?
The new and the old apr_file_t use the same OS file descriptor. Hence if you read from one the position
in the file also changes for the other. This might not matter in the unbuffered case, but in the buffered
case I would imagine that the buffer and the file descriptor position of apr_file_t which was not used for reading
get out of sync and end up in a mess.
Regards
Rüdiger
>
> Modified:
> apr/apr/trunk/file_io/os2/filedup.c
> apr/apr/trunk/file_io/unix/filedup.c
> apr/apr/trunk/file_io/win32/filedup.c
>
> Modified: apr/apr/trunk/file_io/os2/filedup.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/os2/filedup.c?rev=1893204&r1=1893203&r2=1893204&view=diff
> ==============================================================================
> --- apr/apr/trunk/file_io/os2/filedup.c (original)
> +++ apr/apr/trunk/file_io/os2/filedup.c Fri Sep 10 00:51:53 2021
> @@ -113,14 +113,12 @@ APR_DECLARE(apr_status_t) apr_file_setas
> }
>
> if (!(old_file->flags & APR_FOPEN_NOCLEANUP)) {
> + apr_pool_cleanup_kill(old_file->pool, (void *)old_file,
> + apr_file_cleanup);
> apr_pool_cleanup_register(p, (void *)(*new_file),
> apr_file_cleanup,
> apr_file_cleanup);
> }
>
> - old_file->filedes = -1;
> - apr_pool_cleanup_kill(old_file->pool, (void *)old_file,
> - apr_file_cleanup);
> -
> return APR_SUCCESS;
> }
>
> Modified: apr/apr/trunk/file_io/unix/filedup.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/filedup.c?rev=1893204&r1=1893203&r2=1893204&view=diff
> ==============================================================================
> --- apr/apr/trunk/file_io/unix/filedup.c (original)
> +++ apr/apr/trunk/file_io/unix/filedup.c Fri Sep 10 00:51:53 2021
> @@ -179,6 +179,8 @@ APR_DECLARE(apr_status_t) apr_file_setas
> (*new_file)->fname = apr_pstrdup(p, old_file->fname);
> }
> if (!(old_file->flags & APR_FOPEN_NOCLEANUP)) {
> + apr_pool_cleanup_kill(old_file->pool, (void *)old_file,
> + apr_unix_file_cleanup);
> apr_pool_cleanup_register(p, (void *)(*new_file),
> apr_unix_file_cleanup,
> ((*new_file)->flags & APR_INHERIT)
> @@ -186,9 +188,6 @@ APR_DECLARE(apr_status_t) apr_file_setas
> : apr_unix_child_file_cleanup);
> }
>
> - old_file->filedes = -1;
> - apr_pool_cleanup_kill(old_file->pool, (void *)old_file,
> - apr_unix_file_cleanup);
> #ifndef WAITIO_USES_POLL
> (*new_file)->pollset = NULL;
> #endif
>
> Modified: apr/apr/trunk/file_io/win32/filedup.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/filedup.c?rev=1893204&r1=1893203&r2=1893204&view=diff
> ==============================================================================
> --- apr/apr/trunk/file_io/win32/filedup.c (original)
> +++ apr/apr/trunk/file_io/win32/filedup.c Fri Sep 10 00:51:53 2021
> @@ -211,15 +211,13 @@ APR_DECLARE(apr_status_t) apr_file_setas
> (*new_file)->fname = apr_pstrdup(p, old_file->fname);
> }
> if (!(old_file->flags & APR_FOPEN_NOCLEANUP)) {
> + apr_pool_cleanup_kill(old_file->pool, (void *)old_file,
> + file_cleanup);
> apr_pool_cleanup_register(p, (void *)(*new_file),
> file_cleanup,
> file_cleanup);
> }
>
> - old_file->filehand = INVALID_HANDLE_VALUE;
> - apr_pool_cleanup_kill(old_file->pool, (void *)old_file,
> - file_cleanup);
> -
> #if APR_FILES_AS_SOCKETS
> /* Create a pollset with room for one descriptor. */
> /* ### check return codes */
>
>
>