Page MenuHomePhabricator

ecore_con : Fix class check to check inner_socket class
ClosedPublic

Authored by jsuya on May 16 2018, 5:35 AM.

Details

Summary

cl->socket is a Efl.Net.Socket_Simple. it is not inherit the Efl.Loop.fd class.
and The target of the class check have to be the inner_socket.
But inner_socket is Efl.Net.Socket_Ssl. Efl.Net.Socket_Ssl class is not inherit FD class.
Efl.Net.Socket_Tcp is inherit Efl.Loop.fd class. So, Need to add Efl.Net.Socket_Tcp to inheritance.
(The server side is a similar hierarchy. (ssl -> tcp -> ip -> fd))

Test Plan

N/A

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jsuya created this revision.May 16 2018, 5:35 AM
jsuya requested review of this revision.May 16 2018, 5:35 AM
jsuya updated this revision to Diff 14635.May 16 2018, 5:39 AM

fix typo

zmike requested changes to this revision.May 16 2018, 8:15 AM

The method renames should be in a separate patch.

This revision now requires changes to proceed.May 16 2018, 8:15 AM
jsuya updated this revision to Diff 14650.May 16 2018, 9:57 PM
jsuya edited the summary of this revision. (Show Details)

re-write summary

jsuya added a comment.EditedMay 16 2018, 9:59 PM

Hi zmike.
The Summary is wrong. Modified the summary. Method rename and additional inherit class must be included.
(The reason for changing the name of method is that it conflicts with the properties of the Efl.Net.Socket_Tcp class.)

cedric requested changes to this revision.May 17 2018, 8:53 AM
cedric added inline comments.
src/lib/ecore_con/ecore_con_legacy.c
789–790

As Dialer_SSL inherit from Socket_SSL, with the rest of your change, it is now exposing an EFL_LOOP_FD_CLASS too, if I am not mistaken. In which case you can implement in Dialer_SSL efl_loop_fd_get to directly do the call on inner_socket. The idea being that we keep Dialer_SSL internal implementation more to itself and do not expose it outside when not necessary.

src/lib/ecore_con/efl_net_dialer_ssl.eo
42 ↗(On Diff #14650)

Why the rename ? Shouldn't this just become an inherited function implemented in this class ?

53 ↗(On Diff #14650)

Same as above.

This revision now requires changes to proceed.May 17 2018, 8:53 AM
jsuya updated this revision to Diff 14664.May 18 2018, 2:18 AM

Update code.

jsuya added inline comments.May 18 2018, 2:28 AM
src/lib/ecore_con/ecore_con_legacy.c
789–790

I modified so that SSL_CLASS does not expose LOOP_FD_CLASS.
if socket or dialer is SSL_CLASS, fd_get refers to adopted TCP class.

barbieri added inline comments.May 19 2018, 8:28 AM
src/lib/ecore_con/ecore_con_legacy.c
797

these checks looks inverted... IMO it should be ordered as:

  • check EFL_LOOP_FD_CLASS, if so, return efl_loop_fd_get()
  • check if EFL_NET_SOCKET_SSL_CLASS, if so get from adopted, if it's a EFL_LOOP_FD_CLASS
  • fail if none of those.
2430

same order issue here.

jsuya updated this revision to Diff 14706.May 23 2018, 10:09 PM

update code.

cedric accepted this revision.May 25 2018, 10:06 AM

Seems we are good with this.

This revision was not accepted when it landed; it landed in state Needs Review.May 25 2018, 10:41 AM
This revision was automatically updated to reflect the committed changes.