iptube / SPUDlib

Substrate Protocol for User Datagrams (SPUD) Prototype
BSD 2-Clause "Simplified" License
5 stars 11 forks source link

Add tube manager reference to tube objects. #47

Closed phpHavok closed 9 years ago

phpHavok commented 9 years ago

I have found through writing an application that I needed to know a tube's manager. There is currently no way of doing this, except storing the manager in a static global. This patch adds a tmgr field to each tube that is automatically updated on tube_manager add/remove calls.

I will write unit tests if you all agree this is a good contribution.

hildjj commented 9 years ago

Can you talk more about the use case? I was definitely envisioning a small number of tube managers that were either static, or stored in the per-tube data with tube_set_data if really needed. I don't want everyone to have to take the (admittedly small) memory hit, and I didn't want to have to deal with the extra shutdown issues.

phpHavok commented 9 years ago

So the exact use case is whenever the "data" event fires off, I need to flood the incoming message to all attached tubes. The only problem is, the "data" event comes with tube_event_data, which includes a reference to the tube the data was received on, but not its manager.

If this isn't something you think will come up often enough, I think tube_set_data is a perfectly reasonable alternative. In fact, being able to set arbitrary data like that is powerful. I had overlooked that method before, because the header comment says "Unused." I figured it was partially implemented, so I didn't even look into it further.

You provided good reasons for avoiding this. Another good one is the potential for a user to screw up the reference by calling tube_set_manager themselves. There's always that extra overhead whenever you have a bidirectional reference like that.

hildjj commented 9 years ago

What if we put the tube_manager in the tube_event_data structure?

phpHavok commented 9 years ago

Even better! I'll look into that.