On Sun, Aug 16, 2009 at 5:45 AM, Andre Dieb<andre.mart...@ee.ufcg.edu.br> wrote:
> One more thing,
>
> On Sun, Aug 16, 2009 at 5:26 AM, Andre Dieb <andre.mart...@ee.ufcg.edu.br>
> wrote:
>>
>> Sorry for the HTML! :)
>>
>> On Sat, Aug 15, 2009 at 10:39 PM, Gustavo Sverzut Barbieri
>> <barbi...@profusion.mobi> wrote:
>>>
>>> On Sat, Aug 15, 2009 at 5:55 PM, Andre Dieb<andre.mart...@ee.ufcg.edu.br>
>>> wrote:
>>> > Changes:
>>> >
>>> > Module name is now eina_log (but I didn't delete eina_error yet, as we
>>> > should delete it only when the transition is completed)
>>> > Docs on header and basic doc on .c file, but there's a lot of room for
>>> > doc
>>> > improvement
>>> > Save name on domains
>>> > Clean realloc code
>>> > Keep parsing EINA_LOG_LEVELS even when user types wrong (e.g.
>>> > domain1:level1,domain2:,domain3,domain:5)
>>> > Migrate old global logger code to global logger (i.e. remove old
>>> > deprecated
>>> > functions)
>>>
>>> arghhhhhh... HTML MAIL!
>>>
>>> now to the points
>>>  - _DOM is not a suffix, rather a prefix namespace... like
>>> EINA_LOG_ERR and EINA_LOG_DOM_ERR...
>>>  - I remember Sun compiler barfing at ##__VA_ARGS__, maybe we need to
>>> avoid that? Vincent?? (avoid that is define just as (...) and user is
>>> obligated to give at least one parameter, fmt. That makes things
>>> uglier IMHO)
>>
>> That == define as "..." ? Should it be done with ... ?
>> I'm sorry, I didn't follow.

## is a "hack" to remove the last "," if no arguments are provided.
So these would work similarly

   X(a, ...)  bla(a, ##__VA_ARGS__)
   X(...) bla(__VA_ARGS__)

tests

   X(1, 2, 4)  bla(1, 2, 4)
   X(1) bla(1)


>>>
>>>  - _Eina_Log_Level define a value < 0 (EINA_LOG_LEVEL_MIN =
>>> INT32_MIN) so compilers don't optimize that enum as an unsigned
>>> integer, causing problems for users trying to specify something like
>>> -1 for "more critical"
>>>
>>>  - +typedef int Eina_Log; "too much", I'd use it as integer, no need
>>> to typedef.
>>
>> This typedef was already on the code, it was for Eina_Error (a handle for
>> registered errors). Just remove it ?

argh... if it was there, keep it.


>>>  - keep these as "eina_error", you're fixing error->log, but having
>>> error codes as log codes is too much, like broking and not fixing =)
>>> This holds true for eina_log_msg_register and friends...
>
> Put these eina_error_msg_* functions into eina_log or keep eina_error only
> for that?

keep it for these things


>>> +/**
>>> + * @var EINA_LOG_OUT_OF_MEMORY
>>> + * Log identifier corresponding to a lack of memory.
>>> + */
>>> +EAPI extern Eina_Log EINA_LOG_MSG_OUT_OF_MEMORY;
>>>  - colors should be improved, for example (even the old code is not good)
>>> +static const char *_colors[EINA_LOG_LEVELS] = {
>>> +  EINA_COLOR_YELLOW, // EINA_LOG_LEVEL_CRITICAL
>>> +  EINA_COLOR_RED, // EINA_LOG_LEVEL_ERR
>>> +  EINA_COLOR_YELLOW, // EINA_LOG_LEVEL_WARN
>>> +  EINA_COLOR_NOTHING, // EINA_LOG_LEVEL_INFO
>>> +  EINA_COLOR_GREEN, // EINA_LOG_LEVEL_DBG
>>> +};
>>> for me, the more "red", more dangerous... more "green" better... So I
>>> usually use as rule in my debugs:
>>>   - light (\033[1m) red = critical
>>>   - regular (dark) red = error
>>>   - yellow = warning
>>>   - green = info
>>>   - light blue = debug
>>>   - regular blue = more than debug
>>>
>>> things I told you in previous mails:
>>>
>>>  - trailing whitespaces!!!
>>
>> I couldn't find any trailing whitespaces, could you please point them?

grep -n '[ ]\+$' eina_logging_domains5.diff  | cut -d: -f1 | tr '\n' ','

10,26,450,1314


>>>  - have you tested the parse of broken EINA_LOG_LEVELS? Note this
>>> +       level = strtol((char *)(end + 1), &tmp, 10);
>>> +       if (tmp == (end + 1)) continue;
>>> you do not change start, what happens? infinite loops... you need to
>>> have start = next pair, so need to search for "," and update start to
>>> that.... or add an "end" label right after appending to pending list
>>> and "goto end" instead of continue.
>>>  - eina_log_shutdown() should ignore already deleted entries.
>>>  - eina_log_domain_register() should not free domains, they should be
>>> freed in eina_log_domain_unregister()
>>>
>>>
>>> --
>>> Gustavo Sverzut Barbieri
>>> http://profusion.mobi embedded systems
>>> --------------------------------------
>>> MSN: barbi...@gmail.com
>>> Skype: gsbarbieri
>>> Mobile: +55 (19) 9225-2202
>>
>> Thanks a lot for the patience, I hope I can learn with these mistakes :-).

=)

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: barbi...@gmail.com
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with 
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to