WIP: #6664 fixes for server side external io api #6668
Conversation
|
Can one of the admins verify this patch? |
| @@ -819,8 +819,11 @@ BOOL freerdp_peer_context_new(freerdp_peer* client) | |||
| goto fail_error_description; | |||
| } | |||
|
|
|||
| if (!transport_attach(rdp->transport, client->sockfd)) | |||
| goto fail_transport_attach; | |||
| if (client->sockfd != -1) | |||
There was a problem hiding this comment.
Why not simply add a client->ContextConfigure callback to struct rdp_freerdp_peer and call it here?
If the backend desires to configure something, implement it, otherwise skip it.
(just add the new function pointer at the end of the struct)
There was a problem hiding this comment.
because freerdp_peer_context_new is not internal only call and used by existing implementations, but i think suggested solution will also work.
There was a problem hiding this comment.
additionally it will require to implement duplicated freerdp_peer_context_new in implementation, i am not sure what this is a good idea, or maybe i misunderstand you ?
There was a problem hiding this comment.
one more reason to not add one more callbacks, context creation api already have one callback for additional context initialization from user.
There was a problem hiding this comment.
i think this api can be changed to more convenient and flexible one, but this will break compatibility with existing implementations using this api's
There was a problem hiding this comment.
Ok, the suggestion I tried to explain was add a simple call IFCALLRET(client->ContextConfigure, ret, client, client->context); or similar before any transport_attach et al are called.
That will allow you to set your own transport functions and you can remove these socket checks (that dereference deep into the lower layers)
There was a problem hiding this comment.
ok, but we need somehow to disable socket related calls anyway, i mean calls which is not in switchable api. about transport_attach - just avoiding call transport_attach maybe not most elegant solution, but it will do the job without requirement of additional changes
|
i am just started, this is not ready to be merged yet, it's work in progress as suggested by prefix in PR name. |
to disable internal handling of file descriptor
sss123next
force-pushed
the
externel_io_server_side_fixes
branch
from
aead5ca
to
e451b6e
Compare
this will contain necessary fixes in order server side part of external io api to work properly.