From: Daniel Thompson <daniel.thompson@linaro.org>
To: Sumit Garg <sumit.garg@linaro.org>
Cc: kgdb-bugreport@lists.sourceforge.net, jason.wessel@windriver.com,
dianders@chromium.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kdb: Refactor kdb_defcmd implementation
Date: Fri, 19 Mar 2021 17:17:12 +0000 [thread overview]
Message-ID: <20210319171712.vlkgnmp7cbnayxdn@maple.lan> (raw)
In-Reply-To: <20210309121747.859823-1-sumit.garg@linaro.org>
On Tue, Mar 09, 2021 at 05:47:47PM +0530, Sumit Garg wrote:
> Switch to use kdbtab_t instead of separate struct defcmd_set since
> now we have kdb_register_table() to register pre-allocated kdb commands.
This needs rewriting. I've been struggling for some time to figure out
what it actually means means and how it related to the patch. I'm
starting to conclude that this might not be my fault!
> Also, switch to use a linked list for sub-commands instead of dynamic
> array which makes traversing the sub-commands list simpler.
We can't call these things sub-commands! These days a sub-commands
implies something like `git subcommand` and kdb doesn't have anything
like that.
> +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;
Perhaps this should be more like:
struct defcmd_set {
kdbtab_t cmd;
struct list_head commands;
};
This still gets registers using kdb_register_table() but it keeps the
macro code all in once place:
kdb_register_table(¯o->cmd, 1);
I think that is what I *meant* to suggest ;-) . It also avoids having to
talk about sub-commands! BTW I'm open to giving defcmd_set a better name
(kdb_macro?) but I don't see why we want to give all commands a macro
list.
Daniel.
next prev parent reply other threads:[~2021-03-19 17:17 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-09 12:17 [PATCH] kdb: Refactor kdb_defcmd implementation Sumit Garg
2021-03-19 17:17 ` Daniel Thompson [this message]
2021-03-23 6:13 ` Sumit Garg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210319171712.vlkgnmp7cbnayxdn@maple.lan \
--to=daniel.thompson@linaro.org \
--cc=dianders@chromium.org \
--cc=jason.wessel@windriver.com \
--cc=kgdb-bugreport@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=sumit.garg@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).