Re: [PATCH] kdb: Simplify kdb commands registration

[Doug Anderson]

Hi,

On Tue, Jan 19, 2021 at 2:50 AM Sumit Garg <sumit.garg@xxxxxxxxxx> wrote:
>
> Simplify kdb commands registration via using linked list instead of
> static array for commands storage.
>
> Signed-off-by: Sumit Garg <sumit.garg@xxxxxxxxxx>
> ---
>  kernel/debug/kdb/kdb_main.c    | 78 ++++++++++--------------------------------
>  kernel/debug/kdb/kdb_private.h |  1 +
>  2 files changed, 20 insertions(+), 59 deletions(-)

Wow, nice.  It should have been done this way from the start!  ;-)


> @@ -1011,7 +1005,7 @@ int kdb_parse(const char *cmdstr)
>                 ++argv[0];
>         }
>
> -       for_each_kdbcmd(tp, i) {
> +       list_for_each_entry(tp, &kdb_cmds_head, list_node) {
>                 if (tp->cmd_name) {

So I think here (and elsewhere in this file) you can remove this "if
(...->cmd_name)" stuff now, right?  That was all there because the old
"remove" used to just NULL out the name to handle gaps and that's no
longer possible.  If you are really paranoid you could error-check
kdb_register_flags()


> --- a/kernel/debug/kdb/kdb_private.h
> +++ b/kernel/debug/kdb/kdb_private.h
> @@ -174,6 +174,7 @@ typedef struct _kdbtab {
>         short    cmd_minlen;            /* Minimum legal # command
>                                          * chars required */
>         kdb_cmdflags_t cmd_flags;       /* Command behaviour flags */
> +       struct list_head list_node;

nit: every other entry in this struct has a comment but not yours..


Other than those small nits I think this is a great improvement, thanks!

-Doug