coapp-developers team mailing list archive

[Ted Bullock <tbullock@xxxxxxxxxxx>]

On Wed, Aug 4, 2010 at 8:30 AM, Olaf van der Spek <olafvdspek@xxxxxxxxx> wrote:
> 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.

We haven't specified it.  This is how I have coded in the past.
Personally I don't really care.

>
>> '\\ 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...

Is this really important? Why would we need a new line at the end of the file?

>> chip_core = NULL;
>
> Unnecessary, coapp_core_free already does this (it probably shouldn't)
> and the var goes out of scope anyway.

Just me explicitly tidying up.

> libcoapp/coapp.c:
>> #define _COMPILING_COAPP_H
>
> Shouldn't this be defined on project level?

No, it is included in every internal .c file and is responsible for
making sure that the dll_export/dll_import stuff is correctly
specified.

>> static enum coapp_error coapperr = COAPP_SUCCESS;
>
> What's this?
> Shouldn't globals have a g_ prefix?

I do not believe in hungarian notation.  Also this will go away. It
was a passing thought on how to manage errors, except it isn't thread
safe. I have some other ideas.

> libcoapp/coapp.h:
>> COAPP_DB_CANNOT_OPEN
>
> Might want to put a comma after this so reordering and adding entries is easier.

Sure.  As I mentioned the error handling is not really in place just
yet. This will be properly expanded with the usual song and dance when
I get to it.


> libcoapp/coapp_core.c:
>> if(coapp_db_open(&(c->db)) == COAPP_DB_FAILURE)
>
> Shouldn't this be != COAPP_DB_SUCCESS?

Yes, that is correct. However it will change when I get proper error
handling in.

>> coapp_core_free()
>
>> *core = NULL;
>> *core = NULL;
>
> Why twice?
> Standard free doesn't alter it's argument, should we?

Twice because I wrote it in mcdonalds and made a mistake :)

And you are right, free should probably not null it out, however I do
want it nulled.

> Sorry for all the nitpicking. ;)

Its quite all right, I like delicious nitpicking, it builds character.

-- 
Ted Bullock <tbullock@xxxxxxxxxxx>