On Wed, Jul 13, 2011 at 2:49 PM, Mike Frysinger <[email protected]> wrote:
> On Wednesday, July 13, 2011 14:04:01 Jie Zhang wrote:
>> Sorry I didn't make myself clear. My point is user program can have
>> its own verbose level, which might be different to library. User
>> program will output the error description according to its log
>> convention. So urj_error_describe should just return the error
>> description without the urjtag log level prefix.
>
> ah, ok.  in that case, you're right of course.
>
> patch looks fine in general except:
>  - i see some tabs snuck into log-error.c
>  - i dont think urj_log_error_describe() needs to check urj_log_state.level
> since urj_do_log already does this
>
Good catches! Committed as the attached patch.

Many thanks!

Jie
  * 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_describe): Declare.
  * src/tap/detect.c (urj_tap_detect_parts): Use urj_log_error_describe.
  * src/global/log-error.c: Include <stdbool.h>.
    (log_printf): New.
    (urj_do_log): Print log level prefix for error, warning, or when
    log level is URJ_LOG_LEVEL_DETAIL or lower.
    Print file, line, func when log level is URJ_LOG_LEVEL_DEBUG
    or lower.
    (urj_error_describe): Don't include file, line, or function.
    (urj_log_error_describe): New.
  * src/global/parse.c (urj_parse_stream): Remove log level argument.
    Use urj_log_error_describe.
    (urj_parse_file): Remove log level argument.
    (urj_parse_include): Update use of urj_parse_file.
  * src/apps/jtag/jtag.c (jtag_readline_multiple_commands_support):
    Use urj_log_error_describe.
    (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/parse.h
===================================================================
--- include/urjtag/parse.h.orig	2011-07-13 13:39:38.000000000 -0400
+++ include/urjtag/parse.h	2011-07-13 13:39:43.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-13 13:39:38.000000000 -0400
+++ include/urjtag/log.h	2011-07-13 13:39:43.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,12 @@
  * @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 and call urj_error_reset.
+ */
+void urj_log_error_describe (urj_log_level_t level);
 
 /**
  * Convert the named level into the corresponding urj_log_level_t.
Index: src/tap/detect.c
===================================================================
--- src/tap/detect.c.orig	2011-07-13 13:39:38.000000000 -0400
+++ src/tap/detect.c	2011-07-13 13:39:43.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_describe (URJ_LOG_LEVEL_ERROR);
 #ifdef ENABLE_BSDL
         }
 #endif
Index: src/global/log-error.c
===================================================================
--- src/global/log-error.c.orig	2011-07-13 13:39:38.000000000 -0400
+++ src/global/log-error.c	2011-07-13 15:35:53.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
+        || level <= URJ_LOG_LEVEL_DETAIL)
+        r += log_printf (p, "%s: ", urj_log_level_string (level));
+
+    if (urj_log_state.level <= URJ_LOG_LEVEL_DEBUG)
+        r += log_printf (p, "%s:%i %s(): ", file, line, func);
+
+    va_start (ap, fmt);
+    r += (*p) (fmt, ap);
     va_end (ap);
 
     return r;
@@ -187,20 +212,30 @@
 
     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;
 }
+
+void
+urj_log_error_describe (urj_log_level_t level)
+{
+    if (urj_error_get () == URJ_ERROR_OK)
+        return;
+
+    urj_do_log (level,
+                urj_error_state.file, urj_error_state.line,
+                urj_error_state.function,
+                "%s\n", urj_error_describe ());
+
+    urj_error_reset ();
+}
Index: src/global/parse.c
===================================================================
--- src/global/parse.c.orig	2011-07-13 13:39:38.000000000 -0400
+++ src/global/parse.c	2011-07-13 13:39:43.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_describe (URJ_LOG_LEVEL_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-13 13:39:38.000000000 -0400
+++ src/apps/jtag/jtag.c	2011-07-13 13:39:43.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_describe (URJ_LOG_LEVEL_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_error_describe (URJ_LOG_LEVEL_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_error_describe (URJ_LOG_LEVEL_ERROR);
             }
-            urj_error_reset();
+            else
+                urj_error_reset();
         }
     }
 
Index: src/apps/bsdl2jtag/bsdl2jtag.c
===================================================================
--- src/apps/bsdl2jtag/bsdl2jtag.c.orig	2011-07-13 13:39:38.000000000 -0400
+++ src/apps/bsdl2jtag/bsdl2jtag.c	2011-07-13 13:39:43.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_describe (URJ_LOG_LEVEL_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_describe (URJ_LOG_LEVEL_ERROR);
 
     fclose (jtag_file);
     cleanup (chain);
------------------------------------------------------------------------------
AppSumo Presents a FREE Video for the SourceForge Community by Eric 
Ries, the creator of the Lean Startup Methodology on "Lean Startup 
Secrets Revealed." This video shows you how to validate your ideas, 
optimize your ideas and identify your business strategy.
http://p.sf.net/sfu/appsumosfdev2dev
_______________________________________________
UrJTAG-development mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/urjtag-development

Reply via email to