On 2011/03/14 01:52, Philipp Schafft <l...@lion.leolix.org> wrote: > Please comment the patch and tell us what needs to be changed/updated if > any.
Some pedantic comments: You don't need to check for g_mutex_new() failures. When roar_init() returns NULL, the error must be set, or MPD segfaults (roar_mm_calloc() error handler). roar_finish() does not need to call roar_close(), because MPD guarantees that an output is closed before it is destructed. No need to check for NULL before g_free() (roar_finish()). Similar with g_mutex_free(): at this point in roar_finish(), self->lock cannot be NULL. You could add an assert(self->lock!=NULL) if you want to ensure that. Break long lines (80 columns), some of the error messages are too long. I get three compiler warnings (use --enable-werror!): src/output/roar_plugin.c:284:14: error: assignment discards qualifiers from pointer target type src/output/roar_plugin.c:296:19: error: assignment discards qualifiers from pointer target type src/output/roar_plugin.c:301:19: error: assignment discards qualifiers from pointer target type Looks good overall. If you fix the error bug and the warnings, I'll merge it. Max ------------------------------------------------------------------------------ Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d _______________________________________________ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
