add_activity_info should take a handle, and the person who calls it responsible for validating the room in some way. passing potentially invalid info around in internal API is easy to mess up. _set_activites: passes untrusted handle into _inspect without verification (check for more of these errors?). inspecting a handle should never return null. consider an early return/continue too. _set _current_activity: again, fails to validate given room handle. error should say "can't set an activity as current if you're not announcing it" or something.. inspect_room: assumes handle validity _set_properties: "can't set properties on an activity if you're not announcing it" check_prop_in_old_properties: (maybe?) use tp_strdiff, string GValues can contain NULL ... this file is huge, could we actually split it into buddy info and activity properties? would help ensure that they're not too tightly coupled when we want to consider replacing activity properties with something else. update_activities_properties: use a for loop, then strcmp should cause an early continue; not another load of nesting. it currently uses continue in a while loop without following the next pointer, which will just result in infinite looping. :P gabble_pubsub_event_handler maybe doesn't need a return type, or if it does, it should just return whether or not the event namespace was dispatched to a handler, which is a useful thing to know, not whether the handler was "successful" (you don't know then whether the handler has dispatched a meaningful error or if we should reply somehow)