From: Sumit Garg <sumit.garg@linaro.org>
To: Doug Anderson <dianders@chromium.org>
Cc: kgdb-bugreport@lists.sourceforge.net,
Jason Wessel <jason.wessel@windriver.com>,
Daniel Thompson <daniel.thompson@linaro.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] kdb: Simplify kdb commands registration
Date: Thu, 28 Jan 2021 10:42:18 +0530 [thread overview]
Message-ID: <CAFA6WYMZEU3Sme0GH16i+QHv3uhzSevjpEMzfY8trKvT_gV_Hw@mail.gmail.com> (raw)
In-Reply-To: <CAD=FV=X-rA6F_dS-f7f5WO2SZbVYrrg5A5dOrXe_6FrE=ZhqQg@mail.gmail.com>
On Mon, 25 Jan 2021 at 21:32, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sun, Jan 24, 2021 at 11:06 PM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > Simplify kdb commands registration via using linked list instead of
> > static array for commands storage.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >
> > Changes in v2:
> > - Remove redundant NULL check for "cmd_name".
> > - Incorporate misc. comment.
> >
> > kernel/debug/kdb/kdb_main.c | 119 ++++++++++++-----------------------------
> > kernel/debug/kdb/kdb_private.h | 1 +
> > 2 files changed, 34 insertions(+), 86 deletions(-)
> >
> > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> > index 930ac1b..a0989a0 100644
> > --- a/kernel/debug/kdb/kdb_main.c
> > +++ b/kernel/debug/kdb/kdb_main.c
> > @@ -33,6 +33,7 @@
> > #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>
> > @@ -84,15 +85,8 @@ static unsigned int kdb_continue_catastrophic =
> > static unsigned int kdb_continue_catastrophic;
> > #endif
> >
> > -/* kdb_commands describes the available commands. */
> > -static kdbtab_t *kdb_commands;
> > -#define KDB_BASE_CMD_MAX 50
> > -static int kdb_max_commands = KDB_BASE_CMD_MAX;
> > -static kdbtab_t kdb_base_commands[KDB_BASE_CMD_MAX];
> > -#define for_each_kdbcmd(cmd, num) \
> > - for ((cmd) = kdb_base_commands, (num) = 0; \
> > - num < kdb_max_commands; \
> > - num++, num == KDB_BASE_CMD_MAX ? cmd = kdb_commands : cmd++)
> > +/* kdb_cmds_head describes the available commands. */
> > +static LIST_HEAD(kdb_cmds_head);
> >
> > typedef struct _kdbmsg {
> > int km_diag; /* kdb diagnostic */
> > @@ -921,7 +915,7 @@ int kdb_parse(const char *cmdstr)
> > char *cp;
> > char *cpp, quoted;
> > kdbtab_t *tp;
> > - int i, escaped, ignore_errors = 0, check_grep = 0;
> > + int escaped, ignore_errors = 0, check_grep = 0;
> >
> > /*
> > * First tokenize the command string.
> > @@ -1011,25 +1005,18 @@ int kdb_parse(const char *cmdstr)
> > ++argv[0];
> > }
> >
> > - for_each_kdbcmd(tp, i) {
> > - if (tp->cmd_name) {
> > - /*
> > - * If this command is allowed to be abbreviated,
> > - * check to see if this is it.
> > - */
> > -
> > - if (tp->cmd_minlen
> > - && (strlen(argv[0]) <= tp->cmd_minlen)) {
> > - if (strncmp(argv[0],
> > - tp->cmd_name,
> > - tp->cmd_minlen) == 0) {
> > - break;
> > - }
> > - }
> > -
> > - if (strcmp(argv[0], tp->cmd_name) == 0)
> > + list_for_each_entry(tp, &kdb_cmds_head, list_node) {
> > + /*
> > + * If this command is allowed to be abbreviated,
> > + * check to see if this is it.
> > + */
> > + if (tp->cmd_minlen && (strlen(argv[0]) <= tp->cmd_minlen)) {
> > + if (strncmp(argv[0], tp->cmd_name, tp->cmd_minlen) == 0)
> > break;
> > }
>
> The old code had the same problem, but since you're touching it you could fix?
>
> if (a) {
> if (b)
> break;
> }
>
> ...is the same as:
>
> if (a && b)
> break;
>
Sure, I will fix it in the next version.
> In any case, this looks like quite a nice cleanup, so:
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
Thanks,
Sumit
prev parent reply other threads:[~2021-01-28 5:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-25 7:06 [PATCH v2] kdb: Simplify kdb commands registration Sumit Garg
2021-01-25 16:01 ` Doug Anderson
2021-01-28 5:12 ` Sumit Garg [this message]
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=CAFA6WYMZEU3Sme0GH16i+QHv3uhzSevjpEMzfY8trKvT_gV_Hw@mail.gmail.com \
--to=sumit.garg@linaro.org \
--cc=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 \
/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).