Messages in this thread | | | From | Łukasz Bartosik <> | Date | Thu, 21 Dec 2023 16:25:11 +0100 | Subject | Re: [re: PATCH v2 00/15 - 07/11] dyndbg: __skip_spaces |
| |
pon., 18 gru 2023 o 18:17 Petr Mladek <pmladek@suse.com> napisał(a): > > On Thu 2023-12-07 17:15:10, Jim Cromie wrote: > > This private version treats a comma like a space. > > Please, make it clear that this patch adds a new function. > Also please explain why it is needed and what are the effects. > > > Its not known necessary ATM, but seems prudent. > > It might be that I am not a native speaker but I can't parse > the above sentence at all. > > What is not known? > What is prudent? > > > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > > --- > > lib/dynamic_debug.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > > index a380b8151dd8..c974f6e19ca1 100644 > > --- a/lib/dynamic_debug.c > > +++ b/lib/dynamic_debug.c > > @@ -628,6 +628,14 @@ static int ddebug_change(const struct ddebug_query *query, > > return nfound; > > } > > > > +char *__skip_spaces(const char *str) > > +{ > > + str = skip_spaces(str); > > + if (*str == ',') > > + str = skip_spaces(++str); > > + return (char *)str; > > +} > > The function should not be called "skip_spaces" when it skips also > characters which are not typical white-space characters. > > Also the "__" are usually used the other way around. func() > is usually a wrapper around __func(). > > Please, use another name. If you can't find a good name then > it suggests that the design is bad. The result would likely > be hard to understand and maintain. >
I will update the commit based on your comments.
Thanks, Lukasz
> Best Regards, > Petr > > PS: I am sorry, I do not have time for a proper review. I primary wanted > to check if the patchset somehow affected printk(). And I realized > that I do not understand what the changes are about. The commit > messages and the motivation might make sense only to people who > know the big picture. But I was not able to get the picture > for the changes in dot,comma,space handling.
| |