[Libssh] [PATCH 1/2] Add a generic way to handle sockets asynchronously.

[Aris Adamantiadis]

Dear Alksandar,

I have carefuly read your code and I'm now convinced you're going in 
the good direction.

However, my global feeling is that what you did is too generalistic for 
libssh. I don't want libssh to be the tool used by our users as a main 
loop, because it will never be an optimal tool. We may provide basic 
functionnalities though.

Also, it involves exposing more and more of the API, which also 
involves more headaches when we change something.

Anyway, I like your patch. Here are my recommandations before I merge 
it in HEAD:

1/ the POLL* constants should be moved back in priv.h. We don't want to 
export them, because they might be already defined. Also the whole API 
should be moved in priv.h, because I think it should not be external.
2/ Please move your (very precise !) doxygen comments into the code 
instead of the headers, to follow the current practice.
3/ When I said your code was a little big too generalistic, I mostly 
though of the callback system. It's very well written but we don't need 
all of these features at this moment, because a static callback will be 
enough. I think the code which handle the incoming of data should be a 
method of the SSH_SESSION structure/class. See later how I see the user 
provided callback system.
4/ Is it possible to merge your POLL structure with the already 
existing socket object ? I think there is a 1-1 relation between them, 
and some of the already existing code in socket.c overlaps with yours.
5/ The POLL_CTX name is maybe not the right one. Firstly, after the 
POLL/ssh_socket merge, the name will mismatch, but the "CTX" part 
doesn't make it obvious to me that we are dealing with a global socket 
polling system.
Also, I would like to respect the demeter's law regarding the user 
codes. Demeter tells that you should not call a method of an object 
which is member of yet another object. The could I would like to avoid 
(for users) is:

ssh_poll_ctx_add(ssh_get_socket(session),...);
so, this function should take SSH_SESSION's as parameters. (Obviously, 
I have nothing against an internal function which only takes 
ssh_sockets as parameters).
If we insist into provinding callback to users for their own sockets, 
I'd see a simple function doing
ssh_(choosename)_add_socket(CTX *, socket_t usersocket, callback_t 
callback, void *user);
with callback_t being a function pointer taking a socket_t, a void * 
and a pollevents as parameters.

We would wrap ourselves the socket into a ssh_socket and so on. The 
goal is to provide as few API as possible while still providing the 
maximum of possibilities.

My view is still a little fuzzy (as you might see, my mail is not clear 
as crystal and I changed my mind several times while writing), so 
please discuss what you find relevant.

Thanks again for prividing us your patch, and please excuse me for the 
 >79 chars/line layout, I have yet to understand why thunderbird 
doesn't split the lines as it used to do.

Regards,

Aris