Hi Arvid,
Arvid Picciani wrote:
> the attached patch adds callbacks for recv/send to session, allowing
> libssh2 to be integrated with high level socket tools (in my case Qt).
I have two objections.
The first is minor and is about the unclean whitespace. See below.
The second is that the callbacks should probably be called by
_libssh2_send() and _recv() rather than at every call site. Your
patch duplicates a whole lot of ugly parameters that could instead be
just in one place. Also doing the calls in the lower layer allows
errno fixup in a single location.
This means changing _libssh2_send() and _recv() to take session as a
parameter, which I like because two of their parameters are already
struct members.
Finally I'd like some documentation about the send and recv callbacks
to explain what they can do and what they can not do.
> +++ b/include/libssh2.h
> @@ -166,6 +166,15 @@ typedef long long libssh2_int64_t;
> void **abstract)
> #define LIBSSH2_FREE_FUNC(name) void name(void *ptr, void **abstract)
>
> +/* custom i/o callbacks */
> +#define LIBSSH2_RECV_FUNC(name) ssize_t name(int socket, \
> + void *buffer, size_t length, \
> + int flags, void **abstract);
> +#define LIBSSH2_SEND_FUNC(name) ssize_t name(int socket, \
> + const void *buffer, size_t length,\
> + int flags, void **abstract);
> +
> +
Please make that "Custom I/O callbacks" and I think you added one
blank line too many.
> +++ b/src/session.c
..
> - if (ret < 0)
> + if(session->recv){
..
> + }else{
Please make all your ifs and elses look like the rest of libssh2.
//Peter
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2010-05-21