Re: Use tcdrain() instead of tcflush() to implement FlushFileBuffers() for a COM port.

[Alexandre Julliard]

Wolfgang Walter <wine(a)stwm.de> writes:
> @@ -2749,19 +2752,46 @@ NTSTATUS WINAPI NtFlushBuffersFile( HANDLE hFile, IO_STATUS_BLOCK* IoStatusBlock
>  {
>      NTSTATUS ret;
>      HANDLE hEvent = NULL;
> -
> -    SERVER_START_REQ( flush_file )
> -    {
> -        req->handle = wine_server_obj_handle( hFile );
> -        ret = wine_server_call( req );
> -        hEvent = wine_server_ptr_handle( reply->event );
> +    enum server_fd_type type;
> +    unsigned int options;
> +    int needs_close;
> +    int unix_handle;
> +
> +    ret = server_get_unix_fd( hFile, FILE_WRITE_DATA, &unix_handle,
> +                              &needs_close, &type, &options );
> +    if (ret) return ret;

You probably don't want to fail just because there's no unix fd.
> +    if (ret == STATUS_SUCCESS && IoStatusBlock) {
> +        IoStatusBlock->u.Status = ret;
>      }

This is an unrelated change and should be a separate patch, with tests
(also for failures cases),

-- 
Alexandre Julliard
julliard(a)winehq.org

Use tcdrain() instead of tcflush() to implement FlushFileBuffers() for a COM port.

[Wolfgang Walter]

Am Montag, 26. August 2013, 17:13:46 schrieb Alexandre Julliard:
> Wolfgang Walter <wine(a)stwm.de> writes:
> > @@ -2749,19 +2752,46 @@ NTSTATUS WINAPI NtFlushBuffersFile( HANDLE hFile,
> > IO_STATUS_BLOCK* IoStatusBlock> 
> >  {
> >
> > NTSTATUS ret;
> >      HANDLE hEvent = NULL;
> >
> > -
> > -    SERVER_START_REQ( flush_file )
> > -    {
> > -        req->handle = wine_server_obj_handle( hFile );
> > -        ret = wine_server_call( req );
> > -        hEvent = wine_server_ptr_handle( reply->event );
> > +    enum server_fd_type type;
> > +    unsigned int options;
> > +    int needs_close;
> > +    int unix_handle;
> > +
> > +    ret = server_get_unix_fd( hFile, FILE_WRITE_DATA, &unix_handle,
> > +                              &needs_close, &type, &options );
> > +    if (ret) return ret;
>
> You probably don't want to fail just because there's no unix fd.

What is the correct behaviour if there ist no such handle? Should I only fail 
in the case FD_FILE_SERIAL ?

In this case
	server_get_unix_fd()
and
	if (needs_close) close( unix_handle );

could be moved there, too.
> > +    if (ret == STATUS_SUCCESS && IoStatusBlock) {
> > +        IoStatusBlock->u.Status = ret;
> >
> > }
>
> This is an unrelated change and should be a separate patch, with tests
> (also for failures cases),

Ok. I have to see how to write tests.

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München

Use tcdrain() instead of tcflush() to implement FlushFileBuffers() for a COM port.

[Alexandre Julliard]

Wolfgang Walter <wine(a)stwm.de> writes:
> Am Montag, 26. August 2013, 17:13:46 schrieb Alexandre Julliard:
> > You probably don't want to fail just because there's no unix fd.
>
> What is the correct behaviour if there ist no such handle? Should I only fail 
> in the case FD_FILE_SERIAL ?

You'd want to still try the server call, so that the server can
implement a different behavior if needed.

-- 
Alexandre Julliard
julliard(a)winehq.org

Use tcdrain() instead of tcflush() to implement FlushFileBuffers() for a COM port.

[Wolfgang Walter]

Am Montag, 26. August 2013, 19:55:04 schrieben Sie:
> Wolfgang Walter <wine(a)stwm.de> writes:
> > Am Montag, 26. August 2013, 17:13:46 schrieb Alexandre Julliard:
> > > You probably don't want to fail just because there's no unix fd.
> >
> > What is the correct behaviour if there ist no such handle? Should I only
> > fail in the case FD_FILE_SERIAL ?
>
> You'd want to still try the server call, so that the server can
> implement a different behavior if needed.

So like this?



NTSTATUS WINAPI NtFlushBuffersFile( HANDLE hFile, IO_STATUS_BLOCK* IoStatusBlock )
{
    NTSTATUS ret;
    HANDLE hEvent = NULL;
    enum server_fd_type type;
    unsigned int options;
    int needs_close;
    int unix_handle;

    ret = server_get_unix_fd( hFile, FILE_WRITE_DATA, &unix_handle,
                              &needs_close, &type, &options );

    if (!ret && type == FD_TYPE_SERIAL) {
        TRACE("tcdrain %p (%d ; %d)\n", hFile, unix_handle, needs_close);
        while (tcdrain( unix_handle ) == -1) {
            TRACE("tcdrain %p (%d ; %d) returned -1 (%d)\n", hFile, unix_handle, needs_close, errno);
            if (errno != EINTR) {
                ret = FILE_GetNtStatus();
                break;
            }
        }
        TRACE("tcdrained %p (%d ; %d)\n", hFile, unix_handle, needs_close);
    } else {
        SERVER_START_REQ( flush_file )
        {
            req->handle = wine_server_obj_handle( hFile );
            ret = wine_server_call( req );
            hEvent = wine_server_ptr_handle( reply->event );
        }
        SERVER_END_REQ;
        if (!ret && hEvent)
        {
            ret = NtWaitForSingleObject( hEvent, FALSE, NULL );
            NtClose( hEvent );
        }
    }

    if (needs_close) close( unix_handle );
    return ret;
}





Regards,

Wolfgang Walter


P.S.:

The header of NtFlushBuffersFile says:

 * RETURNS
 *  Success: 0. IoStatusBlock is updated.
 *  Failure: An NTSTATUS error code describing the error.

This was the reason I added

    if (ret == STATUS_SUCCESS && IoStatusBlock) {
        IoStatusBlock->u.Status = ret;
    }

None of the applications we use seems to care, though.
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
Abteilungsleiter IT
Leopoldstraße 15
80802 München