I forgot one more thing, and also was asked on IRC for some clarifications about another, so here I am, replying to myself.
A comment I forgot to make, which is to something cedric mentioned, and is something that I've said a million times already and I'll say it again because it's so damn important and people should stop doing it. Please do not delete objects implicitly. :( Connection is closed? Don't delete the object, I may steal be dealing with it. Let me eo_unref/del it if I'm still the rightful owner of the object, it's my responsibility. Maybe I still want to introspect it before it's gone? It's my decision to make... If you really think this pattern of autodeletion is useful, add a helper function people can add as a callback that just eo_dels an object. It's that simple. As for the clarification, I was asked to explain what I meant by "disagree with the concept". What I meant by that, is a "fatter" wrapper than thin. I just want a very thin wrapper, and I disagree with anything that's fatter. :) -- Tom On 04/08/16 17:24, Tom Hacohen wrote: > Hey Gustavo, > > I am going to give a few comments, though it's really hard to review > like this. Although we are creating API here, we don't really care about > the API, we care about the usage. Which means, I don't care about how > the .eo file looks like, I care about how the .c file that uses it looks > like. You haven't provided that, so it means I have to guess, > misunderstand and work extra hard only to start a discussion that will > include more of the above. I don't really have much experience/knowledge > about this topic, but to me, for efficiency and clarity, it seems like > the best way to approach it would be to have a very simple wrapper > around read()/write() that handles address resolution, timeout, lifetime > and loop integration is enough. I don't think we should care about > binbuf or any other complicated setups, all we should get is a const > string (which is essentially a pointer to the static buffer with which > we called "read"), and then decide what we want to do with it on the > client side. Calling binbuf_append is not much work. > All of this should be with simple eo callbacks. As you pointed out, this > is really a solved problem, I think most (all?) the frameworks and > languages do the basics quite the same. I'd follow suit, because I've > never had a bad experience with any. Keep the basics simple, and then > wrap as needed with a subclass (like for example, the send file cedric > suggested). > > 1. We no longer have a "constructor" just properties we mark as > constructing properties and sometimes are only allowed to set on > creation. In this case, since the real finalization is actually done in > "connect", we don't even need those, and can just have normal code there > that checks for those. > 2. I'd also opt for strings for addresses rather than enum/types and > etc. Feels more correct. > 3. I'd also avoid promises. > 4. As said, I'd keep it simple, so would avoid binbuf, and your blob > suggestion. > 5. There are a few naming issues, some have been pointed out. > > > I can probably find additional specific comments, though to be honest, > as said above, I mostly disagree with the concept, and can't really > comment without seeing user code, rather than API description. > > -- > Tom. > > On 03/08/16 05:41, Gustavo Sverzut Barbieri wrote: >> v2 with some fixes. >> >> The analysis of competitors at the end was omitted and you can find >> the updated GIST (diff/highlight): >> >> https://gist.github.com/barbieri/5bdf6c49b207543d764435d2494361ba >> >> Fixed: >> - changed base class from Eo.Base to Efl.Loop_User >> - Returns instead of @out parameters >> - added "drain" event to BaseSocket >> - fixed naming: local_address -> address_local, same for remote >> - added send_buffer_usage property to BaseSocket >> - added receive_buffer_size and send_buffer_size to BaseSocket >> - added address_connected to Socket (client) >> - added address and client_limit to Server >> - addresses of a socket contain port, thus SocketAddress not just Address >> type. >> >> Pending: >> - Address as strings? >> - Use promise<> for send? >> - Constructors without parameters, use properties and finalize? >> - How to have properties to be usable only in constructor->finalize >> timeline? >> - Drop blob and use binbuf? >> >> >> >> /* >> >> This is a draft Eo model to replace ecore_con_{server,client}. >> >> It will address the same usage, but with a new API that is closer >> to what other projects do, this will allow easier transition to >> EFL, without loosing any feature. >> >> At the of this file you can see a detailed analysis of Ecore_Con.h and >> competitors such as node.js, golang, qt and soletta. >> >> NOTE: SSL and Proxy are left out from the initial discussion, Qt is >> the only one to provide API for proxy and they are unused in our >> repository. SSL is usually handled via a subclass, not an upgrade from >> the existing connection. >> >> NOTE: this Eo is not valid or fully complete, it's used as a starting >> base. >> */ >> >> import eina_types; >> >> struct Efl.Net.Address { >> family: Efl.Net.Family; >> address: union { in, in6, bt... }; >> } >> >> struct Efl.Net.SocketAddress { >> address: Efl.Net.Address; >> port: uint16; >> } >> >> // TODO: string->Efl.Net.Address resolver. >> >> /* see http://solettaproject.github.io/docs/c-api/structsol__blob.html >> * >> * The idea is to allow an immutable memory to be reference counted and have >> * parent. >> * >> * Children reference their parent and releases the referece when they >> * die, this allows "windowing" or "slicing" of memory to be handled >> * easily, such as a subsurface keeping the parent framebuffer alive, >> * or a "line" keeping the backing mmap'ed file. >> * >> * Free function allows custom release, such as unloading an image or >> * munmap of a text file. >> * >> * This plays well with bindings, keep the object alive (inc ref) and >> * once the blob dies release them (dec ref). >> */ >> struct Eina.Blob { >> mem: void_ptr; >> parent: Eina.Blob; >> size: size; >> refcount: int; >> free: Eina_Blob_Free_Cb; >> } >> >> /* >> * Using 'Net' instead of 'Network' since all others use a shorter >> * prefix. Also allows to have a side-by-side implementation without >> * the need to immediately convert the all users and allows changing >> * paradigm of the new class -- of course legacy wrappers will be kept >> * for old users. >> */ >> abstract Efl.Net.BaseSocket (Efl.Loop_User) { >> [[Abstract class representing a network connection socket]] >> >> events { >> received @hot: Eina.Binbuf; [[data is available to read, >> if consumed use eina_binbuf_remove().]] >> sent: Eina.Blob, error; [[data specified by blob was sent, >> if error != 0, contains errno]] >> >> drain: void; [[all pending data was sent.]] >> error: int; [[errno]] >> } >> >> methods { >> send @virtual_pure { >> [[Queue data to be sent to remote]] >> params { >> @in data: Eina.Blob; [[data to queue for sending]] >> } >> >> return: int; [[0 on success, ENOSPC if no space left]] >> >> /* can provide a helper that sends memory, creating the >> blob internally, such as static-const (no free) or >> allocated memory (call free) >> */ >> >> // TODO: promise<size, Eina_Error>? >> } >> >> flush @virtual_pure { >> [[Try to send as much as data without blocking]] >> /* NOTE: changes behavior from current flush() that >> * really blocks! >> */ >> return: bool; [[true if all data was sent]] >> } >> >> @property address_local { >> [[the local IP or unix-local address. Analogous to >> getsockname()]] >> get @virtual_pure { >> } >> values { >> address: Efl.Net.SocketAddress; // TODO: string? >> } >> } >> >> @property address_remote { >> [[the remote IP or unix-local address. Analogous to >> getpeername()]] >> get @virtual_pure { >> } >> values { >> address: Efl.Net.SocketAddress; // TODO: string? >> } >> } >> >> /* TO BE REMOVED: >> * it seems the usage is to steal the fd to do something >> * else or set some flags. If so we could expose some >> * primitives such as fcntl_add(type, flags) and >> * fcntl_del(type, flags) as well as a destructor that >> * will return the stolen fd (destroys the object and >> * keeps fd alive). >> */ >> /* @property internal_fd { ... }*/ >> >> @property connected { >> [[if the socket is still connected.]] >> get @virtual_pure { >> } >> values { >> connected: bool; >> } >> } >> >> @property send_buffer_usage { /* maybe pending_send_bytes? */ >> [[how many bytes are queued to be sent. >> See property send_buffer_size.]] >> get @virtual_pure { >> } >> values { >> bytes: size; >> } >> } >> >> @property receive_buffer_size { >> [[Amount of bytes to use when receiving data. >> 0 is unlimited, > 0 is fixed size.]] >> get @virtual_pure { >> } >> values { >> bytes: size; >> } >> } >> >> @property send_buffer_size { >> [[Amount of bytes to use when sending data. >> 0 is unlimited, >> > 0 is an upper limit of queued bytes before >> send() returns ENOSPC]] >> get @virtual_pure { >> } >> values { >> bytes: size; >> } >> } >> >> @property timeout { /* this usually doesn't change, >> should be constructor-only */ >> [[timeout to close the connection.]] >> values { >> timeout: double; >> } >> } >> } >> } >> >> >> class Efl.Net.Socket (Efl.Net.BaseSocket) { >> [[This is a client socket that is created and connected to a >> specific server given to the constructor]] >> >> /* TODO: if address changes to string, add: resolved */ >> /* events { */ >> /* resolved: Efl.Net.Address; [[the connecting address was >> resolved]] */ >> /* } */ >> >> methods { >> connect { >> [[The constructor that connects the socket to a given server]] >> params { >> @in type: Efl.Net.Socket.Type; >> @in address: Efl.Net.SocketAddress; // TODO: string? >> @in timeout: double; >> } >> } >> >> @property address_connected { >> [[The address used to create and connect this socket. It >> is the unresolved address, see the property address_remote >> for the actual resolved address]] >> get @virtual_pure { >> } >> values { >> address: Efl.Net.SocketAddress; // TODO: string? >> } >> } >> } >> >> constructors { >> .connect; >> } >> } >> >> class Efl.Net.Server (Efl.Loop_User) { >> events { >> connection @hot: Efl.Net.BaseSocket; [[new connection was accepted]] >> error: int; [[errno]] >> /* no disconnect, listen on each individual connection */ >> } >> >> methods { >> constructor { >> params { >> /* these could be converted into a struct to scale >> * gracefully and allow just some members to be set, >> * like only timeout without setting client_limit or >> * reject_excess_clients. >> */ >> @in type: Efl.Net.Socket.Type; >> @in address: Efl.Net.SocketAddress; // TODO: string? >> @in client_limit: int; >> @in reject_excess_clients: bool; >> @in timeout: double; >> @in bind_flags: Efl.Net.Socket.Bind.Flags; /* reuse addr..*/ >> @in accept_flags: Efl.Net.Socket.Accept.Flags; /* just >> SOCK_CLOEXEC */ >> } >> } >> >> @property address { >> [[the local IP or unix-local address used to create and listen.]] >> get @virtual_pure { >> } >> values { >> address: Efl.Net.SocketAddress; // TODO: string? >> } >> } >> >> @property client_limit { >> [[number of concurrent clients accepted by this server]] >> get @virtual_pure { >> } >> values { >> client_limit: int; >> reject_excess_clients: bool; >> } >> } >> >> @property timeout { /* this usually doesn't change, >> should be constructor-only */ >> [[timeout to close the connection.]] >> values { >> timeout: double; >> } >> } >> } >> >> constructors { >> .constructor; >> } >> } >> >> > > > ------------------------------------------------------------------------------ > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
