Message-Id: <20210309121747.859823-1-sumit.garg@linaro.org>
Date:   Tue,  9 Mar 2021 17:47:47 +0530
From:   Sumit Garg <sumit.garg@...aro.org>
To:     kgdb-bugreport@...ts.sourceforge.net
Cc:     daniel.thompson@...aro.org, jason.wessel@...driver.com,
        dianders@...omium.org, linux-kernel@...r.kernel.org,
        Sumit Garg <sumit.garg@...aro.org>
Subject: [PATCH] kdb: Refactor kdb_defcmd implementation

Switch to use kdbtab_t instead of separate struct defcmd_set since
now we have kdb_register_table() to register pre-allocated kdb commands.

Also, switch to use a linked list for sub-commands instead of dynamic
array which makes traversing the sub-commands list simpler.

Suggested-by: Daniel Thompson <daniel.thompson@...aro.org>
Signed-off-by: Sumit Garg <sumit.garg@...aro.org>
---
 kernel/debug/kdb/kdb_main.c    | 136 +++++++++++++++------------------
 kernel/debug/kdb/kdb_private.h |   7 ++
 2 files changed, 70 insertions(+), 73 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 9d69169582c6..2f54e81fd9f7 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -33,7 +33,6 @@
 #include <linux/kallsyms.h>
 #include <linux/kgdb.h>
 #include <linux/kdb.h>
-#include <linux/list.h>
 #include <linux/notifier.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
@@ -678,16 +677,7 @@ static void kdb_cmderror(int diag)
  * Returns:
  *	zero for success, a kdb diagnostic if error
  */
-struct defcmd_set {
-	int count;
-	bool usable;
-	char *name;
-	char *usage;
-	char *help;
-	char **command;
-};
-static struct defcmd_set *defcmd_set;
-static int defcmd_set_count;
+static kdbtab_t *defcmd_set;
 static bool defcmd_in_progress;
 
 /* Forward references */
@@ -695,53 +685,52 @@ static int kdb_exec_defcmd(int argc, const char **argv);
 
 static int kdb_defcmd2(const char *cmdstr, const char *argv0)
 {
-	struct defcmd_set *s = defcmd_set + defcmd_set_count - 1;
-	char **save_command = s->command;
+	struct kdb_subcmd *ks;
+
+	if (!defcmd_set)
+		return KDB_NOTIMP;
+
 	if (strcmp(argv0, "endefcmd") == 0) {
 		defcmd_in_progress = false;
-		if (!s->count)
-			s->usable = false;
-		if (s->usable)
-			/* macros are always safe because when executed each
-			 * internal command re-enters kdb_parse() and is
-			 * safety checked individually.
-			 */
-			kdb_register_flags(s->name, kdb_exec_defcmd, s->usage,
-					   s->help, 0,
-					   KDB_ENABLE_ALWAYS_SAFE);
+		if (!list_empty(&defcmd_set->kdb_scmds_head))
+			kdb_register_table(defcmd_set, 1);
 		return 0;
 	}
-	if (!s->usable)
-		return KDB_NOTIMP;
-	s->command = kcalloc(s->count + 1, sizeof(*(s->command)), GFP_KDB);
-	if (!s->command) {
+
+	ks = kmalloc(sizeof(*ks), GFP_KDB);
+	if (!ks) {
 		kdb_printf("Could not allocate new kdb_defcmd table for %s\n",
 			   cmdstr);
-		s->usable = false;
 		return KDB_NOTIMP;
 	}
-	memcpy(s->command, save_command, s->count * sizeof(*(s->command)));
-	s->command[s->count++] = kdb_strdup(cmdstr, GFP_KDB);
-	kfree(save_command);
+
+	ks->scmd_name = kdb_strdup(cmdstr, GFP_KDB);
+	list_add_tail(&ks->list_node, &defcmd_set->kdb_scmds_head);
+
 	return 0;
 }
 
 static int kdb_defcmd(int argc, const char **argv)
 {
-	struct defcmd_set *save_defcmd_set = defcmd_set, *s;
 	if (defcmd_in_progress) {
 		kdb_printf("kdb: nested defcmd detected, assuming missing "
 			   "endefcmd\n");
 		kdb_defcmd2("endefcmd", "endefcmd");
 	}
 	if (argc == 0) {
-		int i;
-		for (s = defcmd_set; s < defcmd_set + defcmd_set_count; ++s) {
-			kdb_printf("defcmd %s \"%s\" \"%s\"\n", s->name,
-				   s->usage, s->help);
-			for (i = 0; i < s->count; ++i)
-				kdb_printf("%s", s->command[i]);
-			kdb_printf("endefcmd\n");
+		kdbtab_t *kp;
+		struct kdb_subcmd *ks;
+
+		list_for_each_entry(kp, &kdb_cmds_head, list_node) {
+			if (kp->cmd_func == kdb_exec_defcmd) {
+				kdb_printf("defcmd %s \"%s\" \"%s\"\n",
+					   kp->cmd_name, kp->cmd_usage,
+					   kp->cmd_help);
+				list_for_each_entry(ks, &kp->kdb_scmds_head,
+						    list_node)
+					kdb_printf("%s", ks->scmd_name);
+				kdb_printf("endefcmd\n");
+			}
 		}
 		return 0;
 	}
@@ -751,45 +740,42 @@ static int kdb_defcmd(int argc, const char **argv)
 		kdb_printf("Command only available during kdb_init()\n");
 		return KDB_NOTIMP;
 	}
-	defcmd_set = kmalloc_array(defcmd_set_count + 1, sizeof(*defcmd_set),
-				   GFP_KDB);
+	defcmd_set = kzalloc(sizeof(*defcmd_set), GFP_KDB);
 	if (!defcmd_set)
 		goto fail_defcmd;
-	memcpy(defcmd_set, save_defcmd_set,
-	       defcmd_set_count * sizeof(*defcmd_set));
-	s = defcmd_set + defcmd_set_count;
-	memset(s, 0, sizeof(*s));
-	s->usable = true;
-	s->name = kdb_strdup(argv[1], GFP_KDB);
-	if (!s->name)
+
+	defcmd_set->cmd_func = kdb_exec_defcmd;
+	defcmd_set->cmd_minlen = 0;
+	defcmd_set->cmd_flags = KDB_ENABLE_ALWAYS_SAFE;
+	defcmd_set->cmd_name = kdb_strdup(argv[1], GFP_KDB);
+	if (!defcmd_set->cmd_name)
 		goto fail_name;
-	s->usage = kdb_strdup(argv[2], GFP_KDB);
-	if (!s->usage)
+	defcmd_set->cmd_usage = kdb_strdup(argv[2], GFP_KDB);
+	if (!defcmd_set->cmd_usage)
 		goto fail_usage;
-	s->help = kdb_strdup(argv[3], GFP_KDB);
-	if (!s->help)
+	defcmd_set->cmd_help = kdb_strdup(argv[3], GFP_KDB);
+	if (!defcmd_set->cmd_help)
 		goto fail_help;
-	if (s->usage[0] == '"') {
-		strcpy(s->usage, argv[2]+1);
-		s->usage[strlen(s->usage)-1] = '\0';
+	if (defcmd_set->cmd_usage[0] == '"') {
+		strcpy(defcmd_set->cmd_usage, argv[2]+1);
+		defcmd_set->cmd_usage[strlen(defcmd_set->cmd_usage)-1] = '\0';
 	}
-	if (s->help[0] == '"') {
-		strcpy(s->help, argv[3]+1);
-		s->help[strlen(s->help)-1] = '\0';
+	if (defcmd_set->cmd_help[0] == '"') {
+		strcpy(defcmd_set->cmd_help, argv[3]+1);
+		defcmd_set->cmd_help[strlen(defcmd_set->cmd_help)-1] = '\0';
 	}
-	++defcmd_set_count;
+
+	INIT_LIST_HEAD(&defcmd_set->kdb_scmds_head);
 	defcmd_in_progress = true;
-	kfree(save_defcmd_set);
 	return 0;
 fail_help:
-	kfree(s->usage);
+	kfree(defcmd_set->cmd_usage);
 fail_usage:
-	kfree(s->name);
+	kfree(defcmd_set->cmd_name);
 fail_name:
 	kfree(defcmd_set);
 fail_defcmd:
 	kdb_printf("Could not allocate new defcmd_set entry for %s\n", argv[1]);
-	defcmd_set = save_defcmd_set;
 	return KDB_NOTIMP;
 }
 
@@ -804,25 +790,29 @@ static int kdb_defcmd(int argc, const char **argv)
  */
 static int kdb_exec_defcmd(int argc, const char **argv)
 {
-	int i, ret;
-	struct defcmd_set *s;
+	int ret;
+	kdbtab_t *kp;
+	struct kdb_subcmd *ks;
+
 	if (argc != 0)
 		return KDB_ARGCOUNT;
-	for (s = defcmd_set, i = 0; i < defcmd_set_count; ++i, ++s) {
-		if (strcmp(s->name, argv[0]) == 0)
+
+	list_for_each_entry(kp, &kdb_cmds_head, list_node) {
+		if (strcmp(kp->cmd_name, argv[0]) == 0)
 			break;
 	}
-	if (i == defcmd_set_count) {
+	if (list_entry_is_head(kp, &kdb_cmds_head, list_node)) {
 		kdb_printf("kdb_exec_defcmd: could not find commands for %s\n",
 			   argv[0]);
 		return KDB_NOTIMP;
 	}
-	for (i = 0; i < s->count; ++i) {
-		/* Recursive use of kdb_parse, do not use argv after
-		 * this point */
+	list_for_each_entry(ks, &kp->kdb_scmds_head, list_node) {
+		/*
+		 * Recursive use of kdb_parse, do not use argv after this point.
+		 */
 		argv = NULL;
-		kdb_printf("[%s]kdb> %s\n", s->name, s->command[i]);
-		ret = kdb_parse(s->command[i]);
+		kdb_printf("[%s]kdb> %s\n", kp->cmd_name, ks->scmd_name);
+		ret = kdb_parse(ks->scmd_name);
 		if (ret)
 			return ret;
 	}
diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h
index ec91d7e02334..a8bda278c9c1 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -13,6 +13,7 @@
  */
 
 #include <linux/kgdb.h>
+#include <linux/list.h>
 #include "../debug_core.h"
 
 /* Kernel Debugger Command codes.  Must not overlap with error codes. */
@@ -164,6 +165,11 @@ typedef struct _kdb_bp {
 #ifdef CONFIG_KGDB_KDB
 extern kdb_bp_t kdb_breakpoints[/* KDB_MAXBPT */];
 
+struct kdb_subcmd {
+	char    *scmd_name;		/* Sub-command name */
+	struct  list_head list_node;	/* Sub-command node */
+};
+
 /* The KDB shell command table */
 typedef struct _kdbtab {
 	char    *cmd_name;		/* Command name */
@@ -175,6 +181,7 @@ typedef struct _kdbtab {
 	kdb_cmdflags_t cmd_flags;	/* Command behaviour flags */
 	struct list_head list_node;	/* Command list */
 	bool    is_dynamic;		/* Command table allocation type */
+	struct list_head kdb_scmds_head; /* Sub-commands list */
 } kdbtab_t;
 
 extern void kdb_register_table(kdbtab_t *kp, size_t len);
-- 
2.25.1

Powered by blists - more mailing lists