From: Gustavo S. B. <bar...@pr...> - 2009-08-16 14:50:11
|
On Sun, Aug 16, 2009 at 5:45 AM, Andre Dieb<and...@ee...> wrote: > One more thing, > > On Sun, Aug 16, 2009 at 5:26 AM, Andre Dieb <and...@ee...> > wrote: >> >> Sorry for the HTML! :) >> >> On Sat, Aug 15, 2009 at 10:39 PM, Gustavo Sverzut Barbieri >> <bar...@pr...> wrote: >>> >>> On Sat, Aug 15, 2009 at 5:55 PM, Andre Dieb<and...@ee...> >>> 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: bar...@gm... >>> 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: bar...@gm... Skype: gsbarbieri Mobile: +55 (19) 9225-2202 |