Re: [PATCH] Implement BindIoCompletionCallback
Andrey Turkin <[EMAIL PROTECTED]> writes: > It is not used in the function itself, it is only passed to > user-provided callback. So, I see several possibilities: I know it's in the callback, what I meant is that the ntdll function should not be taking a LPOVERLAPPED_COMPLETION_ROUTINE, which is a kernel32 thing, it should be taking some sort of IO_COMPLETION_CALLBACK routine, that uses an IO_STATUS_BLOCK (or probably better a void*, since it's opaque) instead of an OVERLAPPED pointer. -- Alexandre Julliard [EMAIL PROTECTED]
Re: [PATCH] Implement BindIoCompletionCallback
Alexandre Julliard wrote: > Robert Shearman <[EMAIL PROTECTED]> writes: > > >> Andrey Turkin wrote: >> >>> Robert Shearman wrote: >>> >>> Andrey Turkin wrote: > > /** > > *BindIoCompletionCallback (KERNEL32.@) > */ > +extern NTSTATUS WINAPI > RtlSetIoCompletionCallback(HANDLE,LPOVERLAPPED_COMPLETION_ROUTINE,ULONG); > > > > This should go in winternl.h. >>> winternl.h may not depend on winbase.h for some reason, so this would >>> mean either changing prototype to "(HANDLE,LPVOID,ULONG)", which seems >>> wrong, or redefining LPOVERLAPPED_COMPLETION_ROUTINE and >>> OVERLAPPED/LPOVERLAPPED, which seems to be too much duplication. >>> >>> >> I think you should guard the function with an #ifdef __WINE_WINBASE_H >> statement, but I would wait for some other developers to chime in to get >> a consensus before accepting this though. >> > > It doesn't seem right for an ntdll function to use an OVERLAPPED > pointer. Shouldn't it take an IO_STATUS_BLOCK instead? > > It is not used in the function itself, it is only passed to user-provided callback. So, I see several possibilities: 1) change RtlSetIoCompletionCallback to use another callback prototype, with LPVOID or ULONG_PTR or something instead of LPOVERLAPPED 2) leave callback prototype as in BindIoCompletionCallback, and protect RtlSetIoCompletionCallback with #ifdefs as Robert suggested 3) do not implement RtlSetIoCompletionCallback at all and move all code in kernel32
Re: [PATCH] Implement BindIoCompletionCallback
Robert Shearman <[EMAIL PROTECTED]> writes: > Andrey Turkin wrote: >> Robert Shearman wrote: >> >>> Andrey Turkin wrote: >>> /** *BindIoCompletionCallback (KERNEL32.@) */ +extern NTSTATUS WINAPI RtlSetIoCompletionCallback(HANDLE,LPOVERLAPPED_COMPLETION_ROUTINE,ULONG); >>> This should go in winternl.h. >>> >> winternl.h may not depend on winbase.h for some reason, so this would >> mean either changing prototype to "(HANDLE,LPVOID,ULONG)", which seems >> wrong, or redefining LPOVERLAPPED_COMPLETION_ROUTINE and >> OVERLAPPED/LPOVERLAPPED, which seems to be too much duplication. >> > > I think you should guard the function with an #ifdef __WINE_WINBASE_H > statement, but I would wait for some other developers to chime in to get > a consensus before accepting this though. It doesn't seem right for an ntdll function to use an OVERLAPPED pointer. Shouldn't it take an IO_STATUS_BLOCK instead? -- Alexandre Julliard [EMAIL PROTECTED]
Re: [PATCH] Implement BindIoCompletionCallback
Andrey Turkin wrote: > Robert Shearman wrote: > >> Andrey Turkin wrote: >> >>> >>> /** >>> >>> *BindIoCompletionCallback (KERNEL32.@) >>> */ >>> +extern NTSTATUS WINAPI >>> RtlSetIoCompletionCallback(HANDLE,LPOVERLAPPED_COMPLETION_ROUTINE,ULONG); >>> >>> >>> >> This should go in winternl.h. >> > winternl.h may not depend on winbase.h for some reason, so this would > mean either changing prototype to "(HANDLE,LPVOID,ULONG)", which seems > wrong, or redefining LPOVERLAPPED_COMPLETION_ROUTINE and > OVERLAPPED/LPOVERLAPPED, which seems to be too much duplication. > I think you should guard the function with an #ifdef __WINE_WINBASE_H statement, but I would wait for some other developers to chime in to get a consensus before accepting this though. -- Rob Shearman
RE: [PATCH] Implement BindIoCompletionCallback
Andrey Turkin wrote:
>diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c
>index e402fed..7bbb3e2 100644
>--- a/dlls/kernel32/tests/sync.c
>+++ b/dlls/kernel32/tests/sync.c
>@@ -327,7 +327,7 @@ static void test_iocp_callback(void)
> retb = p_BindIoCompletionCallback(hFile, iocp_callback, 0);
> ok(retb == FALSE, "BindIoCompletionCallback succeeded on a file that
wasn't created with FILE_FLAG_OVERLAPPED\n");
> if(retb == FALSE && GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) {
>-todo_wine ok (0, "BindIoCompletionCallback returned
ERROR_CALL_NOT_IMPLEMENTED\n");
>+ok (0, "BindIoCompletionCallback returned
ERROR_CALL_NOT_IMPLEMENTED\n");
> return;
> }
I don't think this check makes much sense after the change anymore. It's at
best a skip() for older Windows versions.
Rolf Kalbermatter
Re: [PATCH] Implement BindIoCompletionCallback
On Dec 18, 2007 3:49 PM, Andrey Turkin <[EMAIL PROTECTED]> wrote: > winternl.h may not depend on winbase.h for some reason, so this would > mean either changing prototype to "(HANDLE,LPVOID,ULONG)", which seems > wrong, or redefining LPOVERLAPPED_COMPLETION_ROUTINE and > OVERLAPPED/LPOVERLAPPED, which seems to be too much duplication. Has anyone looked at the latest WDK to see where this goes? Is it really winternl.h? That header seems to be a bit of a bastard hack Microsoft threw together to shut-up the anti-trust case. These functions and others we have had to add to winternl.h have got to be documented in some other DDK header which we can implement. Thanks -- Steven Edwards "There is one thing stronger than all the armies in the world, and that is an idea whose time has come." - Victor Hugo
Re: [PATCH] Implement BindIoCompletionCallback
Robert Shearman wrote:
> Andrey Turkin wrote:
>>
>> /**
>>
>> *BindIoCompletionCallback (KERNEL32.@)
>> */
>> +extern NTSTATUS WINAPI
>> RtlSetIoCompletionCallback(HANDLE,LPOVERLAPPED_COMPLETION_ROUTINE,ULONG);
>>
>>
>
> This should go in winternl.h.
winternl.h may not depend on winbase.h for some reason, so this would
mean either changing prototype to "(HANDLE,LPVOID,ULONG)", which seems
wrong, or redefining LPOVERLAPPED_COMPLETION_ROUTINE and
OVERLAPPED/LPOVERLAPPED, which seems to be too much duplication.
>
>> BOOL WINAPI BindIoCompletionCallback( HANDLE FileHandle,
>> LPOVERLAPPED_COMPLETION_ROUTINE Function, ULONG Flags)
>> {
>> -FIXME("%p, %p, %d, stub!\n", FileHandle, Function, Flags);
>> -SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
>> +NTSTATUS status;
>> +
>> +TRACE("(%p, %p, %d)\n", FileHandle, Function, Flags);
>> +
>> +status = RtlSetIoCompletionCallback( FileHandle, Function, Flags );
>> +if (status == STATUS_SUCCESS) return TRUE;
>> +SetLastError( RtlNtStatusToDosError(status) );
>> return FALSE;
>> }
>
>
Re: [PATCH] Implement BindIoCompletionCallback
Andrey Turkin wrote:
>
> /**
> * BindIoCompletionCallback (KERNEL32.@)
> */
> +extern NTSTATUS WINAPI
> RtlSetIoCompletionCallback(HANDLE,LPOVERLAPPED_COMPLETION_ROUTINE,ULONG);
>
This should go in winternl.h.
> BOOL WINAPI BindIoCompletionCallback( HANDLE FileHandle,
> LPOVERLAPPED_COMPLETION_ROUTINE Function, ULONG Flags)
> {
> -FIXME("%p, %p, %d, stub!\n", FileHandle, Function, Flags);
> -SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
> +NTSTATUS status;
> +
> +TRACE("(%p, %p, %d)\n", FileHandle, Function, Flags);
> +
> +status = RtlSetIoCompletionCallback( FileHandle, Function, Flags );
> +if (status == STATUS_SUCCESS) return TRUE;
> +SetLastError( RtlNtStatusToDosError(status) );
> return FALSE;
> }
>
--
Rob Shearman
