On 9/10/21 2:51 AM, yla...@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 */
>
>
>
