Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 26 Feb 2018 19:09:27 +0200 | Subject | Re: [PATCH 3/4] auxdisplay: charlcd: fix x/y address commands |
| |
On Mon, Feb 26, 2018 at 6:54 PM, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > On Mon, Feb 26, 2018 at 12:44 PM, Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: >> On Mon, Feb 26, 2018 at 1:54 AM, Robert Abel <rabel@robertabel.eu> wrote:
>>> + if ('x' == cmd) { >>> + if (kstrtoul(esc, 10, &tmp_addr.x) < 0) >>> break; >> >>> + } else if ('y' == cmd) { >>> + if (kstrtoul(esc, 10, &tmp_addr.y) < 0) >>> break; >> >> Perhaps instead of dancing around kstrtox() better to switch to >> simple_strtoul() ? > > It seems deprecated: > > /* Obsolete, do not use. Use kstrto<foo> instead */ > extern unsigned long simple_strtoul(const char *,char **,unsigned int);
It has been discussed several times. The comment is simple wrong.
Because of the requirement of kstrtox() to have a \0 or \n followed by \0 as "end of field". simple_strto*() is suitable to be run in place.
>>> } >>> + } >> >> Same indentation level or my mailer hides this from me? > > It is the same, but it is also how the other 'case's do it -- which in > this case looks just wrong since it is the last one of the switch. I > am not sure what is the preferred way of doing these kind of blocks, > coding-style.rst does not seem to give an example for this case.
Comes to my mind - using }} - putting default in between - ... ?
-- With Best Regards, Andy Shevchenko
| |