On Wed, Aug 4, 2010 at 7:32 AM, Ted Bullock <tbull...@comlore.com> wrote: > Hi Folks, > > I just committed a couple hours of thought towards the coapp-engine to: > > http://bazaar.launchpad.net/~tbullock/coapp-engine/experiment/changes
Hi Ted, Just a few quick comments from a scan of the code. Haven't compiled it yet: chip/chip.c: > if(err = coapp_core_new(&chip_core)) What does the style guide say about a space between if and (? My preference is a space. > '\\ No newline at end of file' Might be a good idea to add this newline to every file. A shame VS doesn't do this by default... > chip_core = NULL; Unnecessary, coapp_core_free already does this (it probably shouldn't) and the var goes out of scope anyway. libcoapp/coapp.c: > #define _COMPILING_COAPP_H Shouldn't this be defined on project level? > static enum coapp_error coapperr = COAPP_SUCCESS; What's this? Shouldn't globals have a g_ prefix? libcoapp/coapp.h: > COAPP_DB_CANNOT_OPEN Might want to put a comma after this so reordering and adding entries is easier. libcoapp/coapp_core.c: > if(coapp_db_open(&(c->db)) == COAPP_DB_FAILURE) Shouldn't this be != COAPP_DB_SUCCESS? > coapp_core_free() > *core = NULL; > *core = NULL; Why twice? Standard free doesn't alter it's argument, should we? Sorry for all the nitpicking. ;) Greetings, Olaf _______________________________________________ Mailing list: https://launchpad.net/~coapp-developers Post to : coapp-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~coapp-developers More help : https://help.launchpad.net/ListHelp
