Re: [PATCH 3/4] auxdisplay: charlcd: fix x/y address commands

From: Willy Tarreau
Date: Tue Feb 27 2018 - 23:21:46 EST


Hi Robert,

On Wed, Feb 28, 2018 at 12:29:38AM +0100, Robert Abel wrote:
> It is however an edge case that incurs a
> lot of code for little to no functionality.
> I'd much prefer if we broke backwards compatibility here and actually
> only parse the format that is indicated in the comment:
>
> > case 'x': /* gotoxy : LxXXX[yYYY]; */
> > case 'y': /* gotoxy : LyYYY[xXXX]; */
> >
>
> Exactly one x command followed exactly by zero or one y command or
> vice-versa.
>
> If somebody changes their mind during the escape sequence, they can just
> issue a new one instead of appending to the current one.
>
> I'll post an example patch.

I'm sorry but I think that your patch has simply proven that your point
above doesn't stand. Adding 90 lines of code full of strchr, strpbrk and
memcpy to replace 12 trivial lines, while possibly breaking compatibility
isn't considered an improvement. Reducing code is an improvement,
multiplying it by 7 is not, it adds maintenance burden for no benefit.

Let's stick to Miguel's last version. At least now we know that the
alternatives are worse, which is great.

Thanks,
Willy