* Ibukun Olumuyiwa ([EMAIL PROTECTED]) wrote: > On Thu 25 Mar 2004, Corey Donohoe wrote: > > * Ibukun Olumuyiwa ([EMAIL PROTECTED]) wrote: > > > On Thu 25 Mar 2004, Corey Donohoe wrote: > > > > All of these diff's looked pretty good, but ... :) > > > > > > > > You don't ever seem to close the connection when you successfully > > > > authenticate, so the removed entrance_ipc_shutdown() in > > > > entrance_session.c should probably go back in. > > > > > > Ah, right...that shutdown doesn't happen. Will fix. > > Would it be better to return the pointer to the server, and keep it > > encapsulated in the Entrance_Session? That way we can shutdown the ipc > > when we cleanup the session. hmm... > > I'm not sure putting ipc data inside of the session is a good idea. That > could lead to complications later down the road, possibly with Xdmcp and > any other communication issues that may need to be resolved outside the > context of the session.
If entrance always atleast has an Entrance_Session in memory, we COULD nest the initialization and shutdown of the ipc system into the new/free calls to entrance_session. I admittedly have nfc about Xdmcp, and what's necessary there tho. Entrance w/o an Entrance_Session struct doesn't go very far, unless a lot of the data flow gets gutted. Even in that case, the callbacks that require the _session pointer (_entrance_ipc_server_data) won't work anyways. > > > > > > > > What's the purpose of the static _session variable that was added in > > > > entrance_ipc.c ? Would it be better handled as the data variables to > > > > the different IPC_EVENT types ? I'm assuming this is just something > > > > that isn't being used yet, but probably is there for checking things > > > > when the callbacks roll in. > > > > > > > > ecore_event_handler_add(ECORE_IPC_EVENT .., _entrance_ipc..., session); > > > > > > > > > > At the time of entrance_ipc_init(), the session has not yet been > > > initialized ... can't pass an empty pointer. So I have to manually set it > > > afterwards using ipc_session_set(). This approach is still OK since we're > > > using a static variable. That approach was what I wanted to use at first. > > How about keeping the session_set function, and assigning the callbacks > > at that point. Or have the entrance_ipc_init() function also take an > > Entrance_Session parameter? See the attached diff. > > > > Nice idea, but I'm not convinced it's the best way to go for the reason > outlined above ... in other words, I'm not sure the ipc code should be > confined to the session. Think of IPC as a separate module or object that > other modules in entrance (session, auth) can use to communicate with > entranced. When you look at it from a pseudo-object-oriented standpoint, > session-related data should be modified using a publicly available method > (i.e. ipc_session_set()). The use of a static var emphasises the fact that > it is data local to the ipc module/object. Well I wouldn't really call it data that's local to the ipc module, since it's created/destroyed elsewhere. Its (planned) use at certain times(callbacks) makes sense to just feed it as data to those callbacks. The Entrance_Session struct that we allocate is always present, and there is only one per entrance process(singleton sorta). I don't see any reason to duplicate that pointer in another place and keep it static. I understand the subsystem information hiding-esque concept, I just don't think it makes sense here. Entrance_Session is a handle to all of the memory we've allocated in entrance, you can basically get to anything with it. This is true except in the case of the ipc subsystem. From a fresh co (This isn't the perfect way to grep this, but it'll hopefully make sense) [EMAIL PROTECTED]:~/Source/e17/apps/entrance/src/client$ grep static *.[ch] | grep = entrance_ipc.c:static Ecore_Ipc_Server *server = NULL; entrance_ipc.c:static Entrance_Session *_session = NULL; main.c:static Entrance_Session *session = NULL; ... + two lines from entrance_edit.c ... callbacks with extra data parameters are tastey, we should use them. I can patch in the above mentioned ideas. I must seem like a raving idiot here, well... :) __ Corey Donohoe atmos.org ------------------------------------------------------- This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click _______________________________________________ enlightenment-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
