On 04/27/2008 21:31, Gustavo Sverzut Barbieri wrote: > Hi, > > The SoC student for python-eet (Luiz Irber) is doing the project even > without Google's money,
FWIW... A Google t-shirt can still be obtained for these efforts.
(Will not be an SoC shirt, but a Google t-shirt.) :)
> 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.
>
--
Regards,
Ravenlock
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------- 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
