On Friday, June 17, 2011 15:35:51 Jie Zhang wrote:
> On Fri, Jun 17, 2011 at 1:02 PM, Mike Frysinger wrote:
> > On Friday, June 17, 2011 08:41:35 Jie Zhang wrote:
> >> +    if (level == URJ_LOG_LEVEL_WARNING)
> >> +        msg_header = "Warning";
> >> +    else if (level == URJ_LOG_LEVEL_ERROR)
> >> +        msg_header = "Error";
> >> +    else
> >> +        msg_header = "";
> > 
> > so the msg_header is one of {"Warning", "Error", ""} ...
> 
> I thought this piece of code when I'm writing it but has no good idea
> how to deal with it. I think urj_error_describe should only be called
> for error messages. But there is one and only one place in UrJTAG
> calling it for warning message. I don't know how to not do that for
> that warning message. So urj_error_describe should only be called for
> warning or error. Maybe I should use an assert under the else branch?
> What do you think?

i dont like assert's in libraries.  the current code works fine when called at 
any time.

this translating of level into a string is already handled in 
src/cmd/cmd_debug.c:log_level_to_string().  so why dont we rip those to helper 
funcs out into src/global/log-error.c, make them into proper urj_* funcs, and 
then we can have urj_error_describe() use that.  that way we always get a 
proper prefixed message (since the helper returns "unknown" in the default 
case).

> I'm not in favor of the error reporting system currently used in
> UrJTAG. It's too complicated for me. For example,
> 
> 1. I cannot find a suitable errnum for some error messages in ICE-100B
> cable. I thought adding a new errnum like URJ_ERROR_ICE_100B. But does
> that means we need a number for each cable driver?

we can add a general URJ_ERROR_CABLE if there's something missing.  what 
exactly is the case you're handling that the current URJ_ERROR_* dont cover ?

> 2. urj_error_set/urj_error_describe is difficult to use. Sometimes I
> don't want my error message to show file/line/function before it. It's
> not useful to user and makes the error message read strange. But it's
> not easy to disable this.

i'm fine with changing the file/line/function to only show when the debug 
level is set to detail or debug levels.  the default is normal.
jtag> debug
Current log level is 'normal'

> 3. currently the error messages are handled using two ways. One is
> output immediately. The other is to store it and output late. I don't
> know when to use which, or we should always use the latter, but there
> are some many code using the former now.

my understanding is that core pieces log while cmd pieces output.  that means 
cable drivers should log only.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security 
threats, fraudulent activity, and more. Splunk takes this data and makes 
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
UrJTAG-development mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/urjtag-development

Reply via email to