On Mon, Apr 28, 2008 at 8:57 AM, Cedric BAIL <[EMAIL PROTECTED]> wrote:
> On Mon, Apr 28, 2008 at 4:31 AM, Gustavo Sverzut Barbieri
> <[EMAIL PROTECTED]> wrote:
> > 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.
>
>
> > 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);
>
> This should be part of a big refactor of eet code.
yes, but this don't need to be part of a big refactor, it can come in
small patches by GSoC students! :-)
> > 6 - from 4 and 5 it also look that we should keep the string size
> > around and use it instead of these str* variants...
>
> I already tracked all source of slow down inside eet_data due to str*,
> i left the one that doesn't impact performance. But I am definitively
> thinking about keeping the string size around, this should be also
> part of a refactor.
I know keeping the size may not pay off sometimes, specially with
smaller strings. But again, one looking at it and checking where is
desirable to keep an integer with this is a good thing to do. Again, a
great job for students, we can help by reviewing the patches.
> > 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...
>
> This should be also part of refactoring the code.
again, this can be a small patch for today's EET and done by our dear
students. As raster pointed out at IRC we may have just 5 of these,
but yet, a student can just move this to a function, test and provide
the patch... maybe it will bring us developers more work to explain
how to provide a good patch than to fix it ourselves, but at least it
will get more people involved in 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.
>
> I was thinking about adding a way for some generic pretty printer, so
> it would be easy to write another kind of output. Like a json output,
> or maybe directly the javascript internal object for me. This kind of
> improvement would also help writing support in other scripting langage
> when you want to do eet->script objects.
>
> So if people agree, I am thinking we can plan to :
> - Add unit test and covering to eet with cutest
> - Refactorise/clean code of eet_data
> - Add array support to eet_data
> - Add "generic" pretty printer for script binding (use it later to
> provide undump) to eet_data
> - Add optional support for symetric crypto support per section in
> eet_lib (could be usefull for private config)
>
> This could be the todo for version 2.0. I will not start working on
> this until 1 or 2 months, so if others want to help. But first make a
> 1.0.1 with all small fix before we start breaking thing and add new
> features.
yeah, I discussed this a bit at IRC (but you were away), the idea was
to split the decoder walker from the part that construct the data or
output the contents. In a simple implementation we can just provide a
callback to be called with type, group_type, data and size. Then even
the C implementation could be build on top of it, also the
text-dump... and of course Python or JavaScript or Ruby :-)
--
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
