Subject: Re: Callback for channel data ready

Re: Callback for channel data ready

From: Tom Weber <tom.weber_at_cryptzone.com>
Date: Fri, 16 Mar 2012 12:13:50 +0100

On Thu, March 15, 2012 10:54, Daniel Stenberg wrote:
> It is never too late! We can "easily" add a function today that just reads
> everything off the socket, and then you can use the existing
> libssh2_channel_read() function just fine (when this API "mode" is used, we
> would prevent the channel read functions to try reading anything from the
> transport). We could also add a function that can return exactly which
> channels that have data pending.
>
> Or am I missing something?
>

But how is that "mode" going to be set? Perhaps in the same manner as
blocking, as in:

libssh2_session_set_automatic_read()
libssh2_session_get_automatic_read()

Where my suggested mode would be a zero value.

Speaking of blocking/nonblocking, the API confuses me when it comes to
nonblocking mode and a complex operation returning EAGAIN. Say that I call
libssh2_channel_direct_tcpip_ex() (which involves both a send and receive
operation) and it returns EAGAIN, then I wonder:

1) Did EAGAIN happen during send or receive? Do I need to care?
2) Do I have to make the same call again until the operation is complete?
3) If yes, do I have to pass in the same arguments?
4) If yes, what happens if I pass in other arguments?
5) If yes, what happens if I start some other operation before this one is
complete?

From glancing at the code, I guess the answers are:

1) Don't care
2) Yes
3) Yes
4) Depends
5) ???

I think that either the documentation should be clear in this regard (there
are no pages about the behavior in general), or a clearer asynchronous API
should be implemented, and a synchronous one which builds on that. For
instance:

libssh2_channel_direct_tcpip_async()

Which would return directly, only putting a packet in an out-queue. Then you
would need to keep calling:

libssh2_session_process()

Which both sends and receives as much as possible. It would return an array of
flags, such as:

...REQUEST_COMPLETE
...CHANNEL_DATA_RECEIVED
...NEED_POLLOUT

Where ...REQUEST_COMPLETE signals that the outstanding request (for
simplicity, only one at a time should be allowed) has completed. You would
also get the return code for that request, either through the same call or
through:

libssh2_session_get_request_status()

or similar.

This would probably be too complicated for simple applications, therefore the
asynchronous API should be wrapped inside synchronous helper functions such as
(pseudocode):

int libssh2_channel_direct_tcpip_sync(...)
{
    int flags;
    int rc;

    libssh2_channel_direct_tcpip_async(...);
    do {
        poll(...);
        libssh2_session_process(..., &flags);
    } while (!(flags & LIBSSH2_REQUEST_COMPLETED));

    return libssh2_session_get_request_status(...);
}

It lacks proper error handling and poll direction checking, but it should
illustrate what I mean. Additional benefits of (a)synchronous APIs would be:

1) No need for libssh2_session_set_blocking(), the meaning of which hasn't
been entirely clear for me.
2) No need to switch between blocking and nonblocking mode on the socket
behind the scenes. Just let it always be nonblocking.
3) An opportunity to get rid of BLOCK_ADJUST(), which I think makes the code
significantly harder to read for a newcomer (it did for me).

-- 
Best regards,
Tom Weber
Cryptzone Group AB
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2012-03-16