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

Reply via email to