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

[Aris Adamantiadis <aris@xxxxxxxxxxxx>]

Good luck for your exams. I'm just sorry that you won't continue the
travel with us. I will consider boost as an option for future
development since it's light and may be standardized in the future.
Your patch has been merged in head with a few modifications. Thanks for
this.

Regards,

Aris
Aleksandar Kanchev a écrit :
> Why is glib so scary? :)
>
> You could even download precompiled windows binaries from glib's site
> <http://www.gtk.org/download-windows.html>. Since glib will be mostly
> used inside libssh the API won't change that much, hence nobody should
> be scared. At most someone would need to call g_main_loop_new() and
> g_main_loop_run()
> <http://library.gnome.org/devel/glib/2.20/glib-The-Main-Event-Loop.html>.
> I agree that gobject might make things more complex, but we could
> start with just glib <http://library.gnome.org/devel/glib/2.20/>, see
> how things go and decide later if gobject is really needed or an extra
> gobject binding will suffice.
>
> The pros of using glib is that it offers some nice things, like lists,
> arrays, main loop, io channels and multi-threaded support. Stuffs that
> are needed by libssh and stuff you don't have to worry if they are
> implemented right, since that's the idea of a library. People are
> using libssh for the very same reason.
>
> The cons are that it adds extra library dependency, but this is a
> small issue since glib could be packaged with libssh's windows binary,
> the same way pidgin does it. I don't think that disk size or memory
> requirements aren't an issue since even Intel's new connection manager
> <http://connman.net> uses glib and it's targeted for embedded devices.
>
> I hope that you're also interested in expanding libssh to support more
> and more features. If that's true, then using external libraries will
> definitively help. I'm not saying that every new library should be a
> hard dependency, but since glib is a core library, then it should be.
> You could of course dismiss new libraries and implement the stuff
> yourself but really, what's the point of reinventing the wheel? Why
> not concentrate in expanding the ssh functionality instead? Are really
> that many (windows) users that require this effort? Can't those users
> continue using the "old" libssh? Wouldn't there be even more users if
> the library "plays well"?
>
> Btw I really don't have the time to modify the ssh_poll patch since I
> have to prepare for some big exams. I'm also not that interested in
> going into that direction since it adds more non-ssh related stuff
> (fat) to the library that needs to be maintained. Please, feel free to
> modify it anyway you want.
>
> best regards,
> Aleks
>
>
> On 07/02/2009 10:47 PM, Aris Adamantiadis wrote:
>> Aleksandar,
>>
>> I will lie if I never though of that possibility. I have even though
>> of rewriting libssh in C++ using a decent OOP model.
>>
>> But there is the problem of dependancies which unfortunately are
>> really a problem for a small project like libssh. If we play well,
>> libssh could be integrated in major IO toolkits like glibnet, QT,
>> apache IO library, others, ...
>>
>> Doing the bindings to glib will only make a few people happy and will
>> scare to hell the other people who don't want to force users to
>> install glib only for ssh support (Ask anyone using libssh on windows).
>>
>> I think we need to find the lowest denominator on all IO main loop
>> providers and provind the appropriate binding for them, while
>> providing our own simplified main loop for people wanting something
>> simple.
>> In opposition to what you told, I think it is not that hard to
>> integrate to other main loop while still providing all capabilities.
>>
>> Others, please tell me what you think. I really feel that putting a
>> hard dependance on glib at this state of the development will not
>> help us.
>>
>> About your idea of implementation : I feel we are thinking in the
>> same direction, even if I think your SSH_POLL should be merged with
>> the already existing struct ssh_socket.
>>
>> Do you have somewhere near hand the specifications of the minimal
>> interfaces to be provided to glib for a new network layer ? We'd do a
>> top-down redesign to implement the interfaces to be sure to be
>> compatible, while providing our own main loop system.
>> We will then think on how to convert the existing code to the new
>> interface, little bit by little bit while maintaining the old API
>> integrity. It's important because otherwise incompatible code in a
>> cousin branch will soon be incompatible with the main branch (as we
>> did with the -nonblocking branch years ago).
>>
>> I really count on your previous patch with the several modifications
>> I told you about in my last mail. If needed we'll modifiy it again
>> later.
>>
>> Thanks for your contribution,
>>
>> Aris
>>
>> Aleksandar Kanchev a écrit :
>>> Hi Aris,
>>>
>>> sorry for my late response, it was a busy week(end). Thanks for your
>>> feedback!
>>>
>>> I also think that the patch is too general and exports too much. The
>>> Idea was that we have 3 basic components:
>>>
>>>    1. bind (server) / connect (client) - it just polls the socket for
>>>       read/write until a client connects (ready for accept()) or if the
>>>       client code connect()ed.
>>>    2. version - simply send our version string and read the remote
>>>       version string, check also if it's valid.
>>>    3. packet - this simply reads / writes packets. It also encrypts or
>>>       decrypts them based on the session settings. The packets which
>>> are
>>>       read must also be forwarded to their handlers.
>>>
>>> One SSH_POLL_CTX is created for each thread. It could be passed to
>>> one of the ssh_bind_*() functions to decide which thread will wait
>>> for new clients. The ssh_bind code could internally register a new
>>> SSH_POLL for the bind socket and add it to the SSH_POLL_CTX. The
>>> client code (think main()) would simply have to call ssh_poll_ctx()
>>> within it's main loop.
>>>
>>> When a new client arrives, the internal callback of the bind code
>>> will be called and it would accept() the connection and probably
>>> create a SSH_SESSION object (not sure if it would be needed at that
>>> moment). Then the SSH_POLL_CONTEXT along with the client socket (or
>>> SSH_SESSION) could be passed to the version component.
>>>
>>> The version component could register the client socket with it's own
>>> callback to the SSH_POLL_CTX, send libssh's version string and wait
>>> for the client's version string. Eventually the client string will
>>> arrive since the SSH_POLL_CTX is still being polled within the
>>> main() loop. It could be checked if it's supported, then the version
>>> component could free it's resources (for example the SSH_POLL object
>>> won't be needed anymore) and it could pass the socket along with the
>>> SSH_POLL_CTX to the packet layer. Probably it should create a new
>>> SSH_SESSION object before doing that.
>>>
>>> The packet component could use the SSH_SESSION object to know how to
>>> handle newly received packets (encryption and handlers). It could be
>>> a combination of the current socket and packet components. The need
>>> for a separate socket layer in this architecture is very
>>> questionable since it would only be needed here. The version
>>> component could make some use of it but the version's job is so
>>> trivial that it would probably be an overkill.
>>>
>>> After having these 3 basic components the rest of the code would be
>>> very straight forward: just implement the rest of the ssh protocol.
>>> This would be done by registering handlers for the supported packet
>>> types. The code already exist within libssh, but it probably needs
>>> to be rearranged.
>>>
>>> So, this sounds very nice and easy if you're going to use libssh to
>>> simply implement a ssh client or server. But how about integrating
>>> it with a GUI application. It cannot really coexist with glib's main
>>> loop for example, since glib's loop could also be polling some
>>> sockets or file descriptors. A workaround would be to make the
>>> ssh_poll* code to integrate with glib's main loop. But this would
>>> make the poll code very complicated.
>>>
>>> After thinking about that, why don't we simply use glib for libssh?
>>> My ssh_poll code is actually glib's main loop with glib's io
>>> channels, if you look closer. We could use glib's lists, buffers,
>>> malloc, io, threads, signals ... This would make libssh's code
>>> smaller and would allow us to concentrate on the ssh protocol itself
>>> and leave most of the rest to glib. Why reinvent the wheel?
>>>
>>> Using glib's signals, for example, is a very common callback
>>> mechanism, exactly what you're looking for. It would also allow us
>>> to do complex things, like chaining the callbacks.
>>>
>>> Also most of libssh's code looks like GObject (SSH_SESSION with all
>>> the set and get methods). Porting it to GObject would allow the use
>>> of all the glib bindings: python, perl ... Having glib as dependancy
>>> isn't a bad thing since most of the projects today use it. Doing
>>> "yum remove glib2" on my Fedora 12 adds 545 packages for removal.
>>>
>>> So what I'm saying is, that we should throw my patch away and start
>>> working on a branch of libssh which is build on top of glib and
>>> gobject. This would probably break the API a lot, but the library is
>>> still in version 0.3 and besides, it's typical for an open source
>>> project to break the API when doing such a big change. If everything
>>> goes well you could start releasing the new branch as libssh 1.0.
>>> This would indicate the use of a new API.
>>>
>>> Hopefully, you're not scared by this idea :)
>>>
>>> best regards,
>>> Aleks
>>>
>>>
>>> On 06/25/2009 10:23 PM, Aris Adamantiadis wrote:
>>>> 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
>>>>
>>>> _______________________________________________
>>>> Libssh mailing list
>>>> Libssh@xxxxxxxxxxx
>>>> http://www.cerkinfo.be/cgi-bin/mailman/listinfo/libssh
>>>>   
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>>
>>> _______________________________________________
>>> Libssh mailing list
>>> Libssh@xxxxxxxxxxx
>>> http://www.cerkinfo.be/cgi-bin/mailman/listinfo/libssh
>>
>