Messages in this thread | | | From | Doug Anderson <> | Date | Tue, 2 Jun 2020 14:32:03 -0700 | Subject | Re: [PATCH v4 1/4] kdb: Re-factor kdb_printf() message write code |
| |
Hi,
On Fri, May 29, 2020 at 4:27 AM Sumit Garg <sumit.garg@linaro.org> wrote: > > Re-factor kdb_printf() message write code in order to avoid duplication > of code and thereby increase readability. > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > --- > kernel/debug/kdb/kdb_io.c | 61 +++++++++++++++++++++++++---------------------- > 1 file changed, 32 insertions(+), 29 deletions(-) > > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c > index 924bc92..e46f33e 100644 > --- a/kernel/debug/kdb/kdb_io.c > +++ b/kernel/debug/kdb/kdb_io.c > @@ -542,6 +542,33 @@ static int kdb_search_string(char *searched, char *searchfor) > return 0; > } > > +static void kdb_io_write(char *cp, int len)
nit: "const char *" just to make it obvious that we don't modify the string?
> +{ > + if (len == 0) > + return;
Remove the above check. It's double-overkill. Not only did you just check in kdb_msg_write() but also the while loop below will do a "no-op" just fine even without your check.
> + > + while (len--) { > + dbg_io_ops->write_char(*cp); > + cp++; > + } > +} > + > +static void kdb_msg_write(char *msg, int msg_len)
nit: "const char *" just to make it obvious that we don't modify the string?
Other than those small things, this looks nice to me. Feel free to add my Reviewed-by tag once small things are fixed.
-Doug
| |