On Tue, Jul 5, 2011 at 6:44 PM, Mike Frysinger <[email protected]> wrote:
> On Tuesday, July 05, 2011 17:48:08 Jie Zhang wrote:
>> 2. Compare this (with the debug level prefix):
>
> this could easily be solved by only showing the prefix if (1) the active level
> is verbose (like with the detail level for file/func/line) or (2) if the
> logged level is warning/error.
>
I thought you wanted the prefix was always on. In this version of the
patch, the prefix is only printed out if the logged level is warning
or error. I don't think the prefix is needed on other levels.
The attached patch also solves the problem 1 I raised in my last email.
> the rest looks fine at a glance ...
>
>> I'm not sure if you still like your idea?
>
> i think it takes more work up front and internally with the log code, but the
> resulting API is easier to use and enforces consistency.
>
The attached new version is much simplified compared to the last
version. It's close to the first version I posted in this thread,
except that "error" and "warning" now comes from the common functions.
I'm satisfied with this version, hope you also think it's acceptable.
Regards,
Jie
* include/urjtag/error.h (urj_error_describe): Remove declaration.
* include/urjtag/parse.h (urj_parse_stream): Update declaration.
(urj_parse_file): Likewise.
* include/urjtag/log.h (urj_do_log): Add file, line, func parameters.
(urj_log): Pass __FILE__, __LINE__, __func__ to urj_do_log.
(urj_warning): Use urj_log.
(urj_log_error): Declare.
(urj_log_warn): Declare.
* src/tap/detect.c (urj_tap_detect_parts): Use urj_log_error.
* src/global/log-error.c: Include <stdbool.h>.
(log_printf): New.
(urj_do_log): Print log level prefix for error and warning.
Print file, line, func when log level is URJ_LOG_LEVEL_DETAIL
or lower.
(urj_error_describe): Make it static. Don't include file, line,
or function.
(urj_log_error_1): New.
(urj_log_error): New.
(urj_log_warning): New.
* src/global/parse.c (urj_parse_stream): Remove log level argument.
(urj_parse_file): Likewise.
(urj_parse_include): Update use of urj_parse_file.
* src/apps/jtag/jtag.c (jtag_readline_multiple_commands_support):
Use urj_log_error
(jtag_parse_rc): Update use of urj_parse_file.
(main): Update use of urj_parse_file and urj_parse_stream.
Use urj_log_warning.
* src/apps/bsdl2jtag/bsdl2jtag.c (main): Use urj_log_error.
Index: include/urjtag/error.h
===================================================================
--- include/urjtag/error.h.orig 2011-07-05 23:59:29.000000000 -0400
+++ include/urjtag/error.h 2011-07-06 00:52:25.000000000 -0400
@@ -142,13 +142,5 @@
* Reset the error state.
*/
void urj_error_reset (void);
-/**
- * The error state in human-readable form.
- *
- * This function is not reentrant.
- *
- * @return a pointer to a static area.
- */
-const char *urj_error_describe (void);
#endif /* URJ_ERROR_H */
Index: include/urjtag/parse.h
===================================================================
--- include/urjtag/parse.h.orig 2011-07-05 23:59:29.000000000 -0400
+++ include/urjtag/parse.h 2011-07-05 23:59:31.000000000 -0400
@@ -63,7 +63,7 @@
* URJ_STATUS_ERROR on error
* URJ_STATUS_QUIT on quit command
*/
-int urj_parse_stream (urj_log_level_t ll, urj_chain_t *chain, FILE *f);
+int urj_parse_stream (urj_chain_t *chain, FILE *f);
/**
* Open the specified file and run through urj_parse_stream().
@@ -73,8 +73,7 @@
* URJ_STATUS_ERROR on error
* URJ_STATUS_QUIT on quit command
*/
-int urj_parse_file (urj_log_level_t ll, urj_chain_t *chain,
- const char *filename);
+int urj_parse_file (urj_chain_t *chain, const char *filename);
/**
* Include a file. Autodetects whether it is a bsdl file or a UrJTAG command
Index: include/urjtag/log.h
===================================================================
--- include/urjtag/log.h.orig 2011-07-05 23:59:29.000000000 -0400
+++ include/urjtag/log.h 2011-07-05 23:59:31.000000000 -0400
@@ -40,16 +40,17 @@
extern urj_log_state_t urj_log_state;
-int urj_do_log (urj_log_level_t level, const char *fmt, ...)
+int urj_do_log (urj_log_level_t level, const char *file, size_t line,
+ const char *func, const char *fmt, ...)
#ifdef __GNUC__
- __attribute__ ((format (printf, 2, 3)))
+ __attribute__ ((format (printf, 5, 6)))
#endif
;
#define urj_log(lvl, ...) \
do { \
if ((lvl) >= urj_log_state.level) \
- urj_do_log (lvl, __VA_ARGS__); \
+ urj_do_log (lvl, __FILE__, __LINE__, __func__, __VA_ARGS__); \
} while (0)
/**
@@ -59,12 +60,16 @@
* @param ... consists of a printf argument set. It needs to start with a
* const char *fmt, followed by arguments used by fmt.
*/
-#define urj_warning(...) \
- do { \
- urj_log (URJ_LOG_LEVEL_WARNING, "%s:%d %s() Warning: ", \
- __FILE__, __LINE__, __func__); \
- urj_log (URJ_LOG_LEVEL_WARNING, __VA_ARGS__); \
- } while (0)
+#define urj_warning(...) urj_log (URJ_LOG_LEVEL_WARNING, __VA_ARGS__)
+
+/**
+ * Print the error set by urj_error_set.
+ */
+void urj_log_error (void);
+/**
+ * Print the error set by urj_error_set as a warning.
+ */
+void urj_log_warning (void);
/**
* Convert the named level into the corresponding urj_log_level_t.
Index: src/tap/detect.c
===================================================================
--- src/tap/detect.c.orig 2011-07-05 23:59:29.000000000 -0400
+++ src/tap/detect.c 2011-07-05 23:59:31.000000000 -0400
@@ -426,11 +426,7 @@
strcpy (part->part, partname);
strcpy (part->stepping, stepping);
if (urj_parse_include (chain, data_path, 0) == URJ_STATUS_FAIL)
- {
- urj_log (URJ_LOG_LEVEL_NORMAL, "Error: %s\n",
- urj_error_describe());
- urj_error_reset();
- }
+ urj_log_error ();
#ifdef ENABLE_BSDL
}
#endif
Index: src/global/log-error.c
===================================================================
--- src/global/log-error.c.orig 2011-07-05 23:59:29.000000000 -0400
+++ src/global/log-error.c 2011-07-06 00:52:41.000000000 -0400
@@ -26,6 +26,7 @@
#include <stdarg.h>
#include <errno.h>
#include <string.h>
+#include <stdbool.h>
#include <urjtag/log.h>
#include <urjtag/error.h>
@@ -59,20 +60,44 @@
return r;
}
-int
-urj_do_log (urj_log_level_t level, const char *fmt, ...)
+static int
+log_printf (int (*p) (const char *, va_list), const char *fmt, ...)
{
va_list ap;
int r;
+ va_start (ap, fmt);
+ r = (*p) (fmt, ap);
+ va_end (ap);
+
+ return r;
+}
+
+int
+urj_do_log (urj_log_level_t level, const char *file, size_t line,
+ const char *func, const char *fmt, ...)
+{
+ int (*p) (const char *fmt, va_list ap);
+ va_list ap;
+ int r = 0;
+
if (level < urj_log_state.level)
return 0;
- va_start (ap, fmt);
if (level < URJ_LOG_LEVEL_WARNING)
- r = urj_log_state.out_vprintf (fmt, ap);
+ p = urj_log_state.out_vprintf;
else
- r = urj_log_state.err_vprintf (fmt, ap);
+ p = urj_log_state.err_vprintf;
+
+ if (level == URJ_LOG_LEVEL_WARNING || level == URJ_LOG_LEVEL_ERROR)
+ {
+ r += log_printf (p, "%s: ", urj_log_level_string (level));
+ if (urj_log_state.level <= URJ_LOG_LEVEL_DETAIL)
+ r += log_printf (p, "%s:%i %s(): ", file, line, func);
+ }
+
+ va_start (ap, fmt);
+ r += (*p) (fmt, ap);
va_end (ap);
return r;
@@ -180,27 +205,54 @@
return "UNDEFINED ERROR";
}
-const char *
+static const char *
urj_error_describe (void)
{
static char msg[URJ_ERROR_MSG_LEN + 1024 + 256 + 20];
if (urj_error_state.errnum == URJ_ERROR_IO)
{
- snprintf (msg, sizeof msg, "%s:%d %s() %s: %s %s",
- urj_error_state.file, urj_error_state.line,
- urj_error_state.function,
- "System error", strerror(urj_error_state.sys_errno),
+ snprintf (msg, sizeof msg, "%s: %s %s",
+ "system error", strerror(urj_error_state.sys_errno),
urj_error_state.msg);
}
else
{
- snprintf (msg, sizeof msg, "%s:%d %s() %s: %s",
- urj_error_state.file, urj_error_state.line,
- urj_error_state.function,
+ snprintf (msg, sizeof msg, "%s: %s",
urj_error_string (urj_error_state.errnum),
urj_error_state.msg);
}
return msg;
}
+
+static void
+urj_log_error_1 (bool warning)
+{
+ urj_log_level_t level;
+
+ if (urj_error_get () == URJ_ERROR_OK)
+ return;
+
+ level = warning ? URJ_LOG_LEVEL_WARNING : URJ_LOG_LEVEL_ERROR;
+
+ if (level >= urj_log_state.level)
+ urj_do_log (level,
+ urj_error_state.file, urj_error_state.line,
+ urj_error_state.function,
+ "%s\n", urj_error_describe ());
+
+ urj_error_reset ();
+}
+
+void
+urj_log_error (void)
+{
+ urj_log_error_1 (false);
+}
+
+void
+urj_log_warning (void)
+{
+ urj_log_error_1 (true);
+}
Index: src/global/parse.c
===================================================================
--- src/global/parse.c.orig 2011-07-05 23:59:29.000000000 -0400
+++ src/global/parse.c 2011-07-05 23:59:31.000000000 -0400
@@ -213,7 +213,7 @@
#endif
int
-urj_parse_stream (urj_log_level_t ll, urj_chain_t *chain, FILE *f)
+urj_parse_stream (urj_chain_t *chain, FILE *f)
{
char *inputline, *p;
size_t len;
@@ -248,8 +248,8 @@
go = urj_parse_line (chain, inputline);
if (go == URJ_STATUS_FAIL)
{
- urj_log (ll, "Error: %s; command '%s'\n", urj_error_describe(), inputline);
- urj_error_reset ();
+ urj_log (URJ_LOG_LEVEL_ERROR, "when parsing command '%s'\n", inputline);
+ urj_log_error ();
}
urj_tap_chain_flush (chain);
}
@@ -261,7 +261,7 @@
}
int
-urj_parse_file (urj_log_level_t ll, urj_chain_t *chain, const char *filename)
+urj_parse_file (urj_chain_t *chain, const char *filename)
{
FILE *f;
int go;
@@ -273,7 +273,7 @@
return URJ_STATUS_FAIL;
}
- go = urj_parse_stream (ll, chain, f);
+ go = urj_parse_stream (chain, f);
fclose (f);
urj_log (URJ_LOG_LEVEL_DEBUG, "File Closed go=%d\n", go);
@@ -332,7 +332,7 @@
else
#endif
{
- r = urj_parse_file (URJ_LOG_LEVEL_NORMAL, chain, filename);
+ r = urj_parse_file (chain, filename);
}
free (path);
Index: src/apps/jtag/jtag.c
===================================================================
--- src/apps/jtag/jtag.c.orig 2011-07-05 23:59:29.000000000 -0400
+++ src/apps/jtag/jtag.c 2011-07-05 23:59:31.000000000 -0400
@@ -247,10 +247,7 @@
r = urj_parse_line (chain, line);
if (r == URJ_STATUS_FAIL)
- {
- urj_log (URJ_LOG_LEVEL_NORMAL, "Error: %s\n", urj_error_describe());
- urj_error_reset ();
- }
+ urj_log_error ();
urj_tap_chain_flush (chain);
@@ -336,7 +333,7 @@
if (!file)
return URJ_STATUS_FAIL;
- go = urj_parse_file (URJ_LOG_LEVEL_DETAIL, chain, file);
+ go = urj_parse_file (chain, file);
free (file);
@@ -485,7 +482,7 @@
return -1;
}
- go = urj_parse_file (URJ_LOG_LEVEL_NORMAL, chain, argv[i]);
+ go = urj_parse_file (chain, argv[i]);
cleanup (chain);
if (go < 0 && go != URJ_STATUS_MUST_QUIT)
{
@@ -507,7 +504,7 @@
printf (_("Out of memory\n"));
return -1;
}
- urj_parse_stream (URJ_LOG_LEVEL_NORMAL, chain, stdin);
+ urj_parse_stream (chain, stdin);
cleanup (chain);
@@ -540,10 +537,7 @@
/* Create ~/.jtag */
if (jtag_create_jtagdir () != URJ_STATUS_OK)
- {
- urj_warning ("%s\n", urj_error_describe());
- urj_error_reset();
- }
+ urj_log_warning ();
/* Parse and execute the RC file */
if (!norc)
@@ -553,10 +547,10 @@
if (urj_error_get() != URJ_ERROR_IO)
{
/* Only warn about RC problems; don't prevent running */
- urj_log (URJ_LOG_LEVEL_NORMAL, "Error: %s\n",
- urj_error_describe());
+ urj_log_warning ();
}
- urj_error_reset();
+ else
+ urj_error_reset();
}
}
Index: src/apps/bsdl2jtag/bsdl2jtag.c
===================================================================
--- src/apps/bsdl2jtag/bsdl2jtag.c.orig 2011-07-05 23:59:29.000000000 -0400
+++ src/apps/bsdl2jtag/bsdl2jtag.c 2011-07-05 23:59:31.000000000 -0400
@@ -68,8 +68,7 @@
chain = urj_tap_chain_alloc ();
if (chain == NULL)
{
- urj_log (URJ_LOG_LEVEL_NORMAL, "Error: %s\n",
- urj_error_describe());
+ urj_log_error ();
return 1;
}
@@ -92,10 +91,7 @@
urj_log_state.out_vprintf = log_to_file;
result = urj_bsdl_read_file (chain, argv[1], URJ_BSDL_MODE_DUMP, NULL);
if (result < 0)
- {
- urj_log (URJ_LOG_LEVEL_ERROR, "Error: %s\n", urj_error_describe());
- urj_error_reset ();
- }
+ urj_log_error ();
fclose (jtag_file);
cleanup (chain);
------------------------------------------------------------------------------
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
