ctdb-logging: Rework debug level parsing

Put declarations into ctdb_logging.h, factor out some common code,
clean up #includes.

Remove the check so see if the 1st character of the debug level is
'-'.  This is wrong, since it is trying to check for a negative
numeric debug level (which is no longer supported) and would need to
be handled in the else anyway.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb/common/cmdline.c

@@ -96,10 +96,8 @@ struct ctdb_context *ctdb_cmdline_init(struct event_context *ev)
 	}
 
 	/* Set the debug level */
-	if (isalpha(ctdb_cmdline.debuglevel[0]) || ctdb_cmdline.debuglevel[0] == '-') { 
-		DEBUGLEVEL = get_debug_by_desc(ctdb_cmdline.debuglevel);
-	} else {
-		DEBUGLEVEL = strtol(ctdb_cmdline.debuglevel, NULL, 0);
+	if (!parse_debug(ctdb_cmdline.debuglevel, &DEBUGLEVEL)) {
+		DEBUGLEVEL = DEBUG_ERR;
 	}
 
 	/* set up the tree to store server ids */
@@ -147,10 +145,8 @@ struct ctdb_context *ctdb_cmdline_client(struct tevent_context *ev,
 	}
 
 	/* Set the debug level */
-	if (isalpha(ctdb_cmdline.debuglevel[0]) || ctdb_cmdline.debuglevel[0] == '-') { 
-		DEBUGLEVEL = get_debug_by_desc(ctdb_cmdline.debuglevel);
-	} else {
-		DEBUGLEVEL = strtol(ctdb_cmdline.debuglevel, NULL, 0);
+	if (!parse_debug(ctdb_cmdline.debuglevel, &DEBUGLEVEL)) {
+		DEBUGLEVEL = DEBUG_ERR;
 	}
 
 	ret = ctdb_socket_connect(ctdb);

ctdb/common/ctdb_logging.c

@@ -17,15 +17,18 @@
    along with this program; if not, see <http://www.gnu.org/licenses/>.
 */
 
-#include "includes.h"
-#include "tdb.h"
-#include "system/time.h"
-#include "../include/ctdb_private.h"
-#include "../include/ctdb_client.h"
+#include <ctype.h>
+#include "replace.h"
+#include "ctdb_logging.h"
 
 const char *debug_extra = "";
 
-struct debug_levels debug_levels[] = {
+struct debug_levels {
+	int32_t	level;
+	const char *description;
+};
+
+static struct debug_levels debug_levels[] = {
 	{DEBUG_ERR,	"ERR"},
 	{DEBUG_WARNING,	"WARNING"},
 	{DEBUG_NOTICE,	"NOTICE"},
@@ -43,18 +46,40 @@ const char *get_debug_by_level(int32_t level)
 			return debug_levels[i].description;
 		}
 	}
-	return "Unknown";
+	return NULL;
 }
 
-int32_t get_debug_by_desc(const char *desc)
+static bool get_debug_by_desc(const char *desc, int32_t *level)
 {
 	int i;
 
 	for (i=0; debug_levels[i].description != NULL; i++) {
 		if (!strcasecmp(debug_levels[i].description, desc)) {
-			return debug_levels[i].level;
+			*level = debug_levels[i].level;
+			return true;
 		}
 	}
 
-	return DEBUG_ERR;
+	return false;
+}
+
+bool parse_debug(const char *str, int32_t *level)
+{
+	if (isalpha(str[0])) {
+		return get_debug_by_desc(str, level);
+	} else {
+		*level = strtol(str, NULL, 0);
+		return get_debug_by_level(*level) != NULL;
+	}
+}
+
+void print_debug_levels(FILE *stream)
+{
+	int i;
+
+	for (i=0; debug_levels[i].description != NULL; i++) {
+		fprintf(stream,
+			"%s (%d)\n",
+			debug_levels[i].description, debug_levels[i].level);
+	}
 }

ctdb/include/ctdb_client.h

@@ -564,15 +564,6 @@ int ctdb_ctrl_getscriptstatus(struct ctdb_context *ctdb,
 		    struct ctdb_scripts_wire **script_status);
 
 
-struct debug_levels {
-	int32_t	level;
-	const char *description;
-};
-extern struct debug_levels debug_levels[];
-
-const char *get_debug_by_level(int32_t level);
-int32_t get_debug_by_desc(const char *desc);
-
 int ctdb_ctrl_stop_node(struct ctdb_context *ctdb, struct timeval timeout, uint32_t destnode);
 int ctdb_ctrl_continue_node(struct ctdb_context *ctdb, struct timeval timeout, uint32_t destnode);
 

ctdb/include/ctdb_logging.h

@@ -20,6 +20,10 @@
 #ifndef _CTDB_LOGGING_H_
 #define _CTDB_LOGGING_H_
 
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+
 extern const char *debug_extra;
 
 enum debug_level {
@@ -34,4 +38,8 @@ enum debug_level {
 #define DEBUG_ALERT DEBUG_ERR
 #define	DEBUG_CRIT  DEBUG_ERR
 
+const char *get_debug_by_level(int32_t level);
+bool parse_debug(const char *str, int32_t *level);
+void print_debug_levels(FILE *stream);
+
 #endif /* _CTDB_LOGGING_H_ */

ctdb/tools/ctdb.c

@@ -4845,11 +4845,17 @@ static int control_getdebug(struct ctdb_context *ctdb, int argc, const char **ar
 		DEBUG(DEBUG_ERR, ("Unable to get debuglevel response from node %u\n", options.pnn));
 		return ret;
 	} else {
+		const char *desc = get_debug_by_level(level);
+		if (desc == NULL) {
+			/* This should never happen */
+			desc = "Unknown";
+		}
 		if (options.machinereadable){
 			printf(":Name:Level:\n");
-			printf(":%s:%d:\n",get_debug_by_level(level),level);
+			printf(":%s:%d:\n", desc, level);
 		} else {
-			printf("Node %u is at debug level %s (%d)\n", options.pnn, get_debug_by_level(level), level);
+			printf("Node %u is at debug level %s (%d)\n",
+			       options.pnn, desc, level);
 		}
 	}
 	return 0;
@@ -4999,34 +5005,18 @@ static int control_setrecmasterrole(struct ctdb_context *ctdb, int argc, const c
  */
 static int control_setdebug(struct ctdb_context *ctdb, int argc, const char **argv)
 {
-	int i, ret;
+	int ret;
 	int32_t level;
 
 	if (argc == 0) {
 		printf("You must specify the debug level. Valid levels are:\n");
-		for (i=0; debug_levels[i].description != NULL; i++) {
-			printf("%s (%d)\n", debug_levels[i].description, debug_levels[i].level);
-		}
-
+		print_debug_levels(stdout);
 		return 0;
 	}
 
-	if (isalpha(argv[0][0]) || argv[0][0] == '-') { 
-		level = get_debug_by_desc(argv[0]);
-	} else {
-		level = strtol(argv[0], NULL, 0);
-	}
-
-	for (i=0; debug_levels[i].description != NULL; i++) {
-		if (level == debug_levels[i].level) {
-			break;
-		}
-	}
-	if (debug_levels[i].description == NULL) {
+	if (!parse_debug(argv[0], &level)) {
 		printf("Invalid debug level, must be one of\n");
-		for (i=0; debug_levels[i].description != NULL; i++) {
-			printf("%s (%d)\n", debug_levels[i].description, debug_levels[i].level);
-		}
+		print_debug_levels(stdout);
 		return -1;
 	}