Messages in this thread | | | From | Miguel Ojeda <> | Date | Tue, 27 Feb 2018 21:56:53 +0100 | Subject | Re: [PATCH RFC] auxdisplay: charlcd: Fix and clean up handling of x/y commands |
| |
On Tue, Feb 27, 2018 at 9:13 PM, Willy Tarreau <w@1wt.eu> wrote: > On Tue, Feb 27, 2018 at 08:32:21PM +0100, Miguel Ojeda wrote: >> The current version is not parsing multiple x/y commands as the code >> originally intended. On top of that, kstrtoul() expects >> NULL-terminated strings. Finally, the code had to do two passes over >> the string, while now only one is done. >> >> Some explanations about the supported syntax are added as well. > > Thanks Miguel for doing this. To be fair, I personally think that the code
You're welcome!
> was significantly complexified compared to the original version, just for > the purpose of using one library function designed to multiply by ten and > add a number. Not to mention that it now requires a copy before parsing.
I agree. I wrote this version like this so that parse_n() can be removed in the future if we have access to an in-place alternative to the kstrtoul*() functions (cf. the discussion in the other thread with Andy); which would take the copy away as well.
> For me it simply means that the initial code change was a wrong idea in > the end. But at least with your version, the bugs introduced by the > previous "fix" should now be gone, which is a good thing.
Thanks! :-) This one has the "advantage" that it only goes by the string once (i.e. no initial strchr()) and that gotoxy() is not called unless the full command is valid (but I am not sure whether this is OK or not for the HW, though).
Cheers, Miguel
> > Thanks, > Willy
| |