Hi Branko,
Branko Čibej wrote:
>Well, the only problem with that patch (or rather, my slightly modified
>version of it) is that it doesn't work for me on Windows XP. The file
>descriptor gets opened just fine, and we no longer have a double-close
>bug -- but for some reason I haven't been able to determine yet, Neon
>can't read from the descriptor.
>
I tested my version of the patch (yours is better) on WinXP, but cannot
confirm your problem. Could you try to get the Win32 errornumbers with
the debugger?
>
>I'm pasting my patch here in the hope that somebody can tell me what I'm
>doing wrong...
>
>(BTW, we really _should_ be checking the return code from
>ne_set_request_body_fd in svn_ra_dav__parsed_request.)
>
>(BTW #2, I think we should introduce two new types into APR:
>apr_intptr_t and apr_uintptr_t. That would eliminate one #if in this patch.)
>
>Index: io.c
>===================================================================
>--- io.c (revision 4676)
>+++ io.c (working copy)
>@@ -1657,9 +1657,10 @@
> close_file_descriptor (void *baton)
> {
> int fd = (int) baton;
>- _close (fd);
>- /* Ignore errors from close, because we can't do anything about them. */
>- return APR_SUCCESS;
>+ if (0 > _close (fd))
>+ return APR_EBADF;
>+ else
>+ return APR_SUCCESS;
> }
> #endif
>
>@@ -1674,23 +1675,30 @@
> #ifndef SVN_WIN32
> *fd_p = fd;
> #else
>- *fd_p = _open_osfhandle ((long) fd, _O_RDWR);
>-
>- /* We must close the file descriptor when the apr_file_t is
>- closed, otherwise we'll run out of them. What happens if the
>- underlyig file handle is closed first is anyone's guess, so
>- the pool cleanup just ignores errors from the close. I hope
>- the RTL frees the FD slot before closing the handle ... */
>- if (*fd_p < 0)
>- status = APR_EBADF;
>+ /* APR's pool cleanup closes the original file handle. Since we
>+ can't meddle with that, and we *do* have to close the file
>+ descriptor (which also closes the underlying handle), we'll
>+ create our file descriptor from a duplicate of the handle. */
>+ HANDLE process = GetCurrentProcess();
>+ HANDLE duplicate;
>+ if (!DuplicateHandle (process, fd, process, &duplicate,
>+ 0, FALSE, DUPLICATE_SAME_ACCESS))
>+ status = apr_get_os_error();
> else
> {
>- /* FIXME: This bit of code assumes that the first element of
>- an apr_file_t on Win32 is a pool. It also assumes an int
>- will fit into a void*. Please, let's get rid of this ASAP! */
>- apr_pool_t *cntxt = *(apr_pool_t**) file;
>- apr_pool_cleanup_register (cntxt, (void*) *fd_p,
>- close_file_descriptor, NULL);
>+#if APR_SIZEOF_VOIDP > 4
>+ apr_int64_t os_handle = (apr_int64_t) duplicate;
>+#else
>+ long os_handle = (long) duplicate;
>+#endif
>+ *fd_p = _open_osfhandle (os_handle, _O_RDWR);
>+ if (*fd_p < 0)
>+ status = APR_EBADF;
>+ else
>+ apr_pool_cleanup_register (apr_file_pool_get (file),
>+ (void*) *fd_p,
>+ close_file_descriptor,
>+ apr_pool_cleanup_null);
> }
> #endif
> }
>
>
>
>
>
Patrick
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Feb 1 15:00:53 2003