On Mon, Feb 26, 2018 at 12:44 PM, Andy Shevchenko
<[email protected]> wrote:
> On Mon, Feb 26, 2018 at 1:54 AM, Robert Abel <[email protected]> wrote:
>> NUL-terminate each individual number to be parsed.
>> To do this, the next command character and a pointer to its argument
>> are found and stored. The command character is then overwritten by NUL
>> before kstr* functions are called on the buffer.
>
> Can we avoid yoda style of programming?
>
>> case 'x': /* gotoxy : LxXXX[yYYY]; */
>> + case 'y': { /* gotoxy : LyYYY[xXXX]; */
>> +
>> + char* nxt_esc;
>> + char nxt_cmd;
>> + char cmd;
>> + struct charlcd_priv_addr tmp_addr;
>> +
>> if (!strchr(esc, ';'))
>> break;
>>
>> + /* sequence is processed whether legal or illegal */
>> + processed = 1;
>> +
>> + /* copy current address to temporary buffer */
>> + tmp_addr = priv->addr;
>> +
>> + nxt_cmd = *esc++;
>> + nxt_esc = esc;
>> +
>> + while ('\0' != *esc) {
>> +
>> + cmd = nxt_cmd;
>> + esc = nxt_esc;
>> + nxt_esc = strpbrk(esc, "xy;");
>> + if (NULL != nxt_esc) {
>> + nxt_cmd = *nxt_esc;
>> + /* terminate current sequence with NUL */
>> + *nxt_esc++ = '\0';
>> + }
>> +
>> + 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);
>
>> } else {
>> + /* break on unknown command or ';' */
>> break;
>> }
>> +
>> }
>>
>> + /* unknown commands in sequence will be followed by at least
>> ';' */
>> + if ('\0' != *esc)
>> + break;
>> +
>> + /* clamp new x/y coordinates */
>> + if (tmp_addr.x >= lcd->width)
>> + tmp_addr.x = lcd->width - 1;
>> + tmp_addr.y %= lcd->height;
>> +
>> + priv->addr = tmp_addr;
>> charlcd_gotoxy(lcd);
>> break;
>
>> }
>> + }
>
> 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.
Cheers,
Miguel
>
> --
> With Best Regards,
> Andy Shevchenko
