On Mon, Apr 28, 2008 at 5:04 PM, Gustavo Sverzut Barbieri
<[EMAIL PROTECTED]> wrote:
> 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! :-)
Yep, it can come with small patch, but I would really like them to
came after 1.0.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...
> >
> > 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.
I personnaly don't care who write the code, well that's not true, if
someone else than me is doing it, and do it well, it's much better :-)
> > > 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 :-)
Yep, that was my idea too. It's good to know that someone else is
going to help on this. I will help reviewing code and answer questions
on this subject.
--
Cedric BAIL
-------------------------------------------------------------------------
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
