ayatana-commits team mailing list archive

["Neil J. Patel" <neil.patel@xxxxxxxxxxxxx>]

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.