On Fri, Jul 1, 2011 at 2:58 PM, Mike Frysinger <[email protected]> wrote:
> independently, i like the idea of removing urj_log_level_t from
> urj_parse_{file,stream}.
>
> as for the rest, i dont think this addresses the last few infos i
> posted. i dont understand why any caller to urj_log ever needs to
> prefix "Error:" or "Warning:" to its strings. it already knows if
> something is an error or warning based on the log level passed into
> it.
>
> this is what i mean:
>
There are two problems I saw when trying out your idea.
1. This
> --- global/log-error.c (revision 1948)
> +++ global/log-error.c (working copy)
> + r = urj_log_state.out_vprintf ("%s: ", urj_log_level_string (level));
and this
> + if (urj_log_state.level < URJ_LOG_LEVEL_DETAIL)
> + r = urj_log_state.out_vprintf (fmt, "%s:%i %s(): ", file,
> line, func);
cannot use out_vprintf. Instead something like out_printf is needed. I
don't know if there is a better way. We may also need to handle
log_to_file in bsdl2jtag.c. But I'm not sure.
2. Compare this (with the debug level prefix):
jtag> cable ICE-100B
normal: ICE-100B firmware version is 2.0.0
jtag> detect
normal: IR length: 5
normal: Chain length: 1
normal: Device Id: 00110010011111001000000011001011 (0x327C80CB)
normal: Manufacturer: Analog Devices, Inc. (0x0CB)
normal: Part(0): BF537 (0x27C8)
normal: Stepping: 3
normal: Filename: /tmp/aaa/share/urjtag/analog/bf537/bf537
warning: untested cable or frequency, set wait_clocks to 21
jtag> initbus bf537_ezkit
jtag> detectflash 0x20000000
normal: Query identification string:
normal: Primary Algorithm Command Set and Control Interface ID Code:
0x0002 (AMD/Fujitsu Standard Command Set)
normal: Alternate Algorithm Command Set and Control Interface ID
Code: 0x0000 (null)
normal: Query system interface information:
normal: Vcc Logic Supply Minimum Write/Erase or Write voltage: 2700 mV
normal: Vcc Logic Supply Maximum Write/Erase or Write voltage: 3600 mV
normal: Vpp [Programming] Supply Minimum Write/Erase voltage: 11500 mV
normal: Vpp [Programming] Supply Maximum Write/Erase voltage: 12500 mV
normal: Typical timeout per single byte/word program: 16 us
normal: Typical timeout for maximum-size multi-byte program: 0 us
normal: Typical timeout per individual block erase: 1024 ms
normal: Typical timeout for full chip erase: 0 ms
normal: Maximum timeout for byte/word program: 256 us
normal: Maximum timeout for multi-byte program: 0 us
normal: Maximum timeout per individual block erase: 8192 ms
normal: Maximum timeout for chip erase: 0 ms
normal: Device geometry definition:
normal: Device Size: 4194304 B (4096 KiB, 4 MiB)
normal: Flash Device Interface Code description: 0x0002 (x8/x16)
normal: Maximum number of bytes in multi-byte program: 1
normal: Number of Erase Block Regions within device: 2
normal: Erase Block Region Information:
normal: Region 0:
normal: Erase Block Size: 8192 B (8 KiB)
normal: Number of Erase Blocks: 8
normal: Region 1:
normal: Erase Block Size: 65536 B (64 KiB)
normal: Number of Erase Blocks: 63
normal: Primary Vendor-Specific Extended Query:
normal: Major version number: 1
normal: Minor version number: 0
normal: Address Sensitive Unlock: Required
normal: Erase Suspend: Read/write
normal: Sector Protect: 1 sectors per group
normal: Sector Temporary Unprotect: Not supported
normal: Sector Protect/Unprotect Scheme: 29BDS640 mode (Software
Command Locking)
normal: Simultaneous Operation: Not supported
normal: Burst Mode Type: Supported
normal: Page Mode Type: Not supported
to this (without the debug level prefix string)
jtag> cable ICE-100B
ICE-100B Firmware Version is 2.0.0
jtag> detect
IR length: 5
Chain length: 1
Device Id: 00110010011111001000000011001011 (0x327C80CB)
Manufacturer: Analog Devices, Inc. (0x0CB)
Part(0): BF537 (0x27C8)
Stepping: 3
Filename: /tmp/aaa/share/urjtag/analog/bf537/bf537
Warning: untested cable or frequency, set wait_clocks to 21
jtag> initbus bf537_ezkit
jtag> detectflash 0x20000000
Query identification string:
Primary Algorithm Command Set and Control Interface ID Code: 0x0002
(AMD/Fujitsu Standard Command Set)
Alternate Algorithm Command Set and Control Interface ID Code: 0x0000
(null)
Query system interface information:
Vcc Logic Supply Minimum Write/Erase or Write voltage: 2700 mV
Vcc Logic Supply Maximum Write/Erase or Write voltage: 3600 mV
Vpp [Programming] Supply Minimum Write/Erase voltage: 11500 mV
Vpp [Programming] Supply Maximum Write/Erase voltage: 12500 mV
Typical timeout per single byte/word program: 16 us
Typical timeout for maximum-size multi-byte program: 0 us
Typical timeout per individual block erase: 1024 ms
Typical timeout for full chip erase: 0 ms
Maximum timeout for byte/word program: 256 us
Maximum timeout for multi-byte program: 0 us
Maximum timeout per individual block erase: 8192 ms
Maximum timeout for chip erase: 0 ms
Device geometry definition:
Device Size: 4194304 B (4096 KiB, 4 MiB)
Flash Device Interface Code description: 0x0002 (x8/x16)
Maximum number of bytes in multi-byte program: 1
Number of Erase Block Regions within device: 2
Erase Block Region Information:
Region 0:
Erase Block Size: 8192 B (8 KiB)
Number of Erase Blocks: 8
Region 1:
Erase Block Size: 65536 B (64 KiB)
Number of Erase Blocks: 63
Primary Vendor-Specific Extended Query:
Major version number: 1
Minor version number: 0
Address Sensitive Unlock: Required
Erase Suspend: Read/write
Sector Protect: 1 sectors per group
Sector Temporary Unprotect: Not supported
Sector Protect/Unprotect Scheme: 29BDS640 mode (Software Command
Locking)
Simultaneous Operation: Not supported
Burst Mode Type: Supported
Page Mode Type: Not supported
Do we really want to prefix with every log level?
And to achieve the above output (with the prefix), I also need to
introduce a new function which can print the log without the prefix,
otherwise the dots when updating the firmware will look like:
normal: .normal: .normal: .normal: .normal: .normal: .normal: .normal:
.normal: .normal: .normal: .normal: .normal: .normal:
We also need to add the interface to allow user of UrJTAG library to
adjust the indent of log message, as I did in the attached patch. This
is needed for gdbproxy to make the log messages from UrJTAG and
gdbproxy use the same indent.
I'm not sure if you still like your idea?
Regards,
Jie
Index: include/urjtag/error.h
===================================================================
--- include/urjtag/error.h.orig 2011-07-05 17:02:27.000000000 -0400
+++ include/urjtag/error.h 2011-07-05 17:24:24.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 17:02:27.000000000 -0400
+++ include/urjtag/parse.h 2011-07-05 17:03:26.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 17:02:27.000000000 -0400
+++ include/urjtag/log.h 2011-07-05 17:10:49.000000000 -0400
@@ -27,6 +27,9 @@
#include "types.h"
+/* The max length of logging level names. */
+#define URJ_LOG_MAX_PREFIX_LENGTH 7
+
/**
* Log state.
*/
@@ -35,12 +38,21 @@
urj_log_level_t level; /**< logging level */
int (*out_vprintf) (const char *fmt, va_list ap);
int (*err_vprintf) (const char *fmt, va_list ap);
+ int (*out_printf) (const char *fmt, ...);
+ int (*err_printf) (const char *fmt, ...);
}
urj_log_state_t;
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, 5, 6)))
+#endif
+ ;
+
+int urj_log_raw (urj_log_level_t level, const char *fmt, ...)
#ifdef __GNUC__
__attribute__ ((format (printf, 2, 3)))
#endif
@@ -49,7 +61,7 @@
#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 +71,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.
@@ -84,4 +100,11 @@
*/
const char *urj_log_level_string (urj_log_level_t level);
+/**
+ * Set the indent of log messages.
+ *
+ * @param indent the indent for log messages
+ */
+void urj_log_set_indent (int indent);
+
#endif /* URJ_LOG_H */
Index: src/tap/detect.c
===================================================================
--- src/tap/detect.c.orig 2011-07-05 17:02:27.000000000 -0400
+++ src/tap/detect.c 2011-07-05 17:03:26.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 17:02:27.000000000 -0400
+++ src/global/log-error.c 2011-07-05 17:24:56.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>
@@ -35,14 +36,21 @@
static int stderr_vprintf (const char *fmt, va_list ap);
static int stdout_vprintf (const char *fmt, va_list ap);
+static int stderr_printf (const char *fmt, ...);
+static int stdout_printf (const char *fmt, ...);
urj_log_state_t urj_log_state =
{
.level = URJ_LOG_LEVEL_NORMAL,
.out_vprintf = stdout_vprintf,
.err_vprintf = stderr_vprintf,
+ .out_printf = stdout_printf,
+ .err_printf = stderr_printf,
};
+/* The indent for log messages. The additional spaces are for ": ". */
+static int urj_log_indent = URJ_LOG_MAX_PREFIX_LENGTH + 2;
+
static int
stderr_vprintf(const char *fmt, va_list ap)
{
@@ -59,8 +67,68 @@
return r;
}
+static int
+stderr_printf(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start (ap, fmt);
+ return vfprintf (stderr, fmt, ap);
+}
+
+static int
+stdout_printf(const char *fmt, ...)
+{
+ va_list ap;
+ int r;
+
+ va_start (ap, fmt);
+ r = vfprintf (stdout, fmt, ap);
+ fflush (stdout);
+
+ return r;
+}
+
int
-urj_do_log (urj_log_level_t level, const char *fmt, ...)
+urj_do_log (urj_log_level_t level, const char *file, size_t line,
+ const char *func, const char *fmt, ...)
+{
+ int (*vp) (const char *fmt, va_list ap);
+ int (*p) (const char *fmt, ...);
+ va_list ap;
+ int r;
+ /* We need additional spaces for ':' and '\0'. */
+ char buffer[URJ_LOG_MAX_PREFIX_LENGTH + 2];
+
+ if (level < urj_log_state.level)
+ return 0;
+
+ if (level < URJ_LOG_LEVEL_WARNING)
+ {
+ vp = urj_log_state.out_vprintf;
+ p = urj_log_state.out_printf;
+ }
+ else
+ {
+ vp = urj_log_state.err_vprintf;
+ p = urj_log_state.err_printf;
+ }
+
+ sprintf (buffer, "%s:", urj_log_level_string (level));
+ r = (*p) ("%*s", - urj_log_indent, buffer);
+
+ if (urj_log_state.level <= URJ_LOG_LEVEL_DETAIL)
+ r = (*p) ("%s:%i %s(): ", file, line, func);
+
+ va_start (ap, fmt);
+ r += (*vp) (fmt, ap);
+ va_end (ap);
+
+ return r;
+}
+
+int
+urj_log_raw (urj_log_level_t level, const char *fmt, ...)
{
va_list ap;
int r;
@@ -90,6 +158,16 @@
urj_error_state.errnum = URJ_ERROR_OK;
}
+void
+urj_log_set_indent (int indent)
+{
+ if (indent < URJ_LOG_MAX_PREFIX_LENGTH + 1)
+ indent = URJ_LOG_MAX_PREFIX_LENGTH + 1;
+ urj_log_indent = indent;
+}
+
+/* URJ_LOG_MAX_PREFIX_LENGTH in log.h might need to be changed
+ if you change the following table. */
static const struct {
const urj_log_level_t level;
const char *name;
@@ -180,27 +258,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 17:02:27.000000000 -0400
+++ src/global/parse.c 2011-07-05 17:03:26.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 17:02:27.000000000 -0400
+++ src/apps/jtag/jtag.c 2011-07-05 17:03:26.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 17:02:27.000000000 -0400
+++ src/apps/bsdl2jtag/bsdl2jtag.c 2011-07-05 17:03:26.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
