I think the discussion in the last thread is a mess now. So I start a
new thread with a new version of the patch.

The previous version of the patch tried to move "Error" and "Warning"
into urj_error_describe, which caused all the discussion about how to
deal with other log levels for the new log level parameter for
urj_error_describe. The version tries to avoid this by not touching
urj_error_describe, but creating two new functions: urj_log_error and
urj_log_warning and letting them output "Error" and "Warning". In such
way, we will not have the problem that what to do if user passes a bad
value to that new parameter.

This version of the patch also simplifies logging of error and warning
in the spirit of Mike's suggestion in the last discussion thread.

It also removes the log level argument from urj_parse_stream and
urj_parse_file. I wrote an email to the original author who introduced
this to ask why these functions need that. But no replies so far. I
know I just wait for 1 day. But I think output error on the error
level is a natural thing. So I just removed the log level argument and
output the errors on the error level.

Looks good now?

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_warning): Make the message start with
    "Warning".
	(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>.
    (urj_error_describe): Make it static.
    (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	(revision 1947)
+++ include/urjtag/error.h	(working copy)
@@ -142,13 +142,5 @@ urj_error_t urj_error_get (void);
  * 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	(revision 1947)
+++ include/urjtag/parse.h	(working copy)
@@ -63,7 +63,7 @@ int urj_parse_line (urj_chain_t *chain,
  *      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 @@ int urj_parse_stream (urj_log_level_t ll
  *      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	(revision 1947)
+++ include/urjtag/log.h	(working copy)
@@ -61,12 +61,21 @@ int urj_do_log (urj_log_level_t level, c
  */
 #define urj_warning(...) \
     do { \
-        urj_log (URJ_LOG_LEVEL_WARNING, "%s:%d %s() Warning: ", \
+        urj_log (URJ_LOG_LEVEL_WARNING, "Warning: %s:%d %s(): ", \
                  __FILE__, __LINE__, __func__); \
         urj_log (URJ_LOG_LEVEL_WARNING, __VA_ARGS__); \
     } while (0)
 
 /**
+ * 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.
  *
  * @param slevel the string to translate
Index: src/tap/detect.c
===================================================================
--- src/tap/detect.c	(revision 1947)
+++ src/tap/detect.c	(working copy)
@@ -426,11 +426,7 @@ urj_tap_detect_parts (urj_chain_t *chain
             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	(revision 1947)
+++ src/global/log-error.c	(working copy)
@@ -26,6 +26,7 @@
 #include <stdarg.h>
 #include <errno.h>
 #include <string.h>
+#include <stdbool.h>
 
 #include <urjtag/log.h>
 #include <urjtag/error.h>
@@ -180,7 +181,7 @@ urj_error_string (urj_error_t err)
     return "UNDEFINED ERROR";
 }
 
-const char *
+static const char *
 urj_error_describe (void)
 {
     static char msg[URJ_ERROR_MSG_LEN + 1024 + 256 + 20];
@@ -204,3 +205,33 @@ urj_error_describe (void)
 
     return msg;
 }
+
+static void
+urj_log_error_1 (bool warning)
+{
+    const char *msg_header;
+    urj_log_level_t level;
+
+    if (urj_error_get () == URJ_ERROR_OK)
+        return;
+
+    level = warning ? URJ_LOG_LEVEL_WARNING : URJ_LOG_LEVEL_ERROR;
+    msg_header = warning ? "Warning" : "Error";
+
+    urj_log (level, "%s: %s\n", msg_header, 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	(revision 1947)
+++ src/global/parse.c	(working copy)
@@ -213,7 +213,7 @@ static ssize_t getline(char **line, size
 #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 @@ urj_parse_stream (urj_log_level_t ll, ur
         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, "Error: when parsing command '%s'\n", inputline);
+            urj_log_error ();
         }
         urj_tap_chain_flush (chain);
     }
@@ -261,7 +261,7 @@ urj_parse_stream (urj_log_level_t ll, ur
 }
 
 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 @@ urj_parse_file (urj_log_level_t ll, urj_
         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 @@ urj_parse_include (urj_chain_t *chain, c
     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	(revision 1947)
+++ src/apps/jtag/jtag.c	(working copy)
@@ -247,10 +247,7 @@ jtag_readline_multiple_commands_support
 
         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 @@ jtag_parse_rc (urj_chain_t *chain)
     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 @@ main (int argc, char *const argv[])
                 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 @@ main (int argc, char *const argv[])
             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 @@ main (int argc, char *const argv[])
 
     /* 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 @@ main (int argc, char *const argv[])
             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	(revision 1947)
+++ src/apps/bsdl2jtag/bsdl2jtag.c	(working copy)
@@ -68,8 +68,7 @@ main (int argc, char *const argv[])
     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 @@ main (int argc, char *const argv[])
     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

Reply via email to