Review: Needs Fixing Looks good, had a couple of points: configure_signals: - I'd normally use slightly more higher-level API such as g_dir_open, g_dir_read_names and g_dir_close, as I think they protect you from some errors (as well as giving you some useful GError's if somethings gone wrong). - Speaking of g_dir_close, I don't see *d being closed :)
add_signal: - I'd use g_strcmp0 instead of strcmp as it'll protect you if name == NULL - g_slist_next just expands to signal->next, so maybe that looks cleaner (this is just style stuff, so ignore me if you want :)? last block of code: - Generally, I'd s/strcmp/g_strcmp0 as mentioned before - If you find the matching string and you delete the link, you should also break out of the forloop as deleting a link half-way through an iteration could invalidate the list pointers (afaik)?. -- https://code.edge.launchpad.net/~bratsche/xsplash/configurable-signals/+merge/11082 Your team ayatana-commits is subscribed to branch lp:xsplash. _______________________________________________ Mailing list: https://launchpad.net/~ayatana-commits Post to : [email protected] Unsubscribe : https://launchpad.net/~ayatana-commits More help : https://help.launchpad.net/ListHelp
