On 03/31/2010 08:43 PM, Pete Zaitcev wrote:
diff --git a/server/server.c b/server/server.c
index 3208e0f..2d68ee6 100644
--- a/server/server.c
+++ b/server/server.c
@@ -55,7 +55,7 @@ static struct argp_option options[] = {
          "Store database environment in DIRECTORY.  Default: "
          CLD_DEF_DATADIR },
        { "debug", 'D', "LEVEL", 0,
-         "Set debug output to LEVEL (0 = off, 2 = max)" },
+         "Set debug output to LEVEL (0 = off, 1 = debugging)" },
        { "stderr", 'E', NULL, 0,
          "Switch the log to standard error" },
        { "foreground", 'F', NULL, 0,
@@ -64,6 +64,8 @@ static struct argp_option options[] = {
          "Bind to UDP port PORT.  Default: " CLD_DEF_PORT },
        { "pid", 'P', "FILE", 0,
          "Write daemon process id to FILE.  Default: " CLD_DEF_PIDFN },
+       { "verbose", 'v', NULL, 0,
+         "Enable the session-level verbosity" },
        { "strict-free", 1001, NULL, 0,
          "For memory-checker runs.  When shutting down server, free local "
          "heap, rather than simply exit(2)ing and letting OS clean up." },


As is hinted by the current code's debugging switch being an integer 'level' value, the server [and client?] has increasing levels of verbosity. The debug levels are

        0: key messages affecting server operation, only
        1: debugging output enabled, sans per-packet output
        2: debugging output enabled, including per-packet output

ie. clearly ordered by increasing value == increased verbosity.

As is clearly illustrated when I cut the patch down to the above snippet, the user interface you have created gives the user two "knobs" for log verbosity, and it is not clear to a casual user which knob controls which sets of messages. That makes for a -more- confusing user interface, because the user must constantly ask themselves the question "do I need debug? or verbose? I don't know!"

Additionally, this interface changes runs counter to other tools, which increase verbosity with added "-v" switches -- analagous to the existing integer-based debug level interface.

If it is truly your desire to permit fine-grained selection of certain classes of messages, then don't dick around! Go ahead and create a bitmap "log mask" which permits fine-grained selection of various messages, much like netif_msg_* and netif_msg_init() in the kernel's include/linux/netdevice.h.

Having two switches, -d and -v, for different, undocumented classes of message just increases confusion. Put yourself in the mind of a user trying to figure out which is which.

I readily admit the __internal implementation__ resulting from your patches is a useful cleanup, but at a macro level, it merely increases logging user interface confusion.

        Jeff


--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to