Hi,

The SoC student for python-eet (Luiz Irber) is doing the project even
without Google's money, so while reading the code in order to do
proper mentoring I found some minor details. If you're ok with them,
the student can provide patches for these and I can commit.

1 - minor spell errors in the docs, easy to fix using aspell/ispell

2 - why descriptor macros don't use offsetof() macros (in GCC one can
do it using __builtin_offsetof()). Vincent, can you confirm if similar
stuff is available on Windows? This would improve readability.

3 - why not make EET_T_* and EET_G_* enums and use them in functions?

4 - in many places we use the construction:
       len = strlen(str);
       strcpy(copy, str);
    why not use memcpy() since we already know the size?

5 - in other places it's bit worse:
     copy = strdup(str);
     len = strlen(str);
   this should be replaced with (will save 1 walk of the whole string):
     len = strlen(str);
     copy = malloc(len + 1);
     memcpy(copy, str, len + 1);

6 - from 4 and 5 it also look that we should keep the string size
around and use it instead of these str* variants...

7 - why endianess/bytesex is detected at runtime?

8 - eet_coder is sometimes accessed with type and others with (type -
1), is this correct? It would be better to convert to the correct (0
or 1 start) and go with it all along. This was a quick look, no real
attempt to understand surrounding code.

9 - minor weirdness in casts... not respecting const where it could
(eet_data_{get,put}_*), unneeded casts for malloc.

10 - some code paths could be split into functions and be reused, to name one:
             if (data)
               {
                  echnk = eet_data_chunk_new(data, size, ede->name,
ede->type, ede->group_type);
                  eet_data_chunk_put(ed, echnk, ds);
                  eet_data_chunk_free(echnk);
                  free(data);
                  data = NULL;
              }
    this is replicated all over the code...

11 - split dump code from decode... at least move the code inside if
(dumpfunc) to another func, it will make code easier to read and still
might make it a bit fast, because most used functions will be smaller.

As you can see it's a bunch of _MINOR_ stuff, very good to get
students used to E code and start contributing!  If you're ok,
students could provide one patch for each item, these would be posted
to the ML and then we can commit.

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi
Embedded Systems
--------------------------------------
MSN: [EMAIL PROTECTED]
Skype: gsbarbieri
Mobile: +55 (81) 9927 0010

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to