On Friday, June 17, 2011 08:41:35 Jie Zhang wrote: > Any comments? If no one objects, I will commit it at the beginning of > the next week.
the basic idea is fine, but some implementation issues ...
> --- src/global/log-error.c (revision 1907)
> +++ src/global/log-error.c (working copy)
>
> + char *msg_header;
const
> + 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", ""} ...
> - snprintf (msg, sizeof msg, "%s:%d %s() %s: %s %s",
> + snprintf (msg, sizeof msg, "%s: %s:%d %s() %s: %s %s",
> + msg_header,
>
> + snprintf (msg, sizeof msg, "%s: %s:%d %s() %s: %s",
> + msg_header,
> + urj_error_state.file, urj_error_state.line,
but you always use "%s: " with msg_header, even when it's "". so perhaps the
": " should be moved into msg_header and the format string changed to just
"%s" rather than "%s: ".
> - urj_log (URJ_LOG_LEVEL_NORMAL, "Error: %s\n", urj_error_describe());
> + urj_log (URJ_LOG_LEVEL_ERROR, "%s\n",
> + urj_error_describe(URJ_LOG_LEVEL_ERROR));
there seems to be a lot of this style now. this brings up two questions ...
do we ever use urj_log() without a trailing new line ? seems like we should
move that into urj_log() itself.
many (most? all?) of our logs are of the form:
"xxxxxx: %s\n", ..., urj_error_describe()
so perhaps it'd make sense to add a bit to the first parameter of urj_log() so
that it'll automatically append the error description. think perror().
and along those lines, if many times we are simply displaying the error, then
perhaps add a helper func like urj_log_simple(level) which expands into:
urj_log(level | URJ_LOG_APPEND_ERROR, "");
-mike
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------------ EditLive Enterprise is the world's most technically advanced content authoring tool. Experience the power of Track Changes, Inline Image Editing and ensure content is compliant with Accessibility Checking. http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________ UrJTAG-development mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/urjtag-development
