Re: [PATCH 34/35] tty: make tty_get_byte_size available

From: Joe Perches
Date: Thu May 06 2021 - 04:36:03 EST


On Thu, 2021-05-06 at 11:24 +0300, Andy Shevchenko wrote:
> On Thursday, May 6, 2021, Joe Perches <joe@xxxxxxxxxxx> wrote:
[]
> > Perhaps clearer not testing account_flags multiple times.
> >
> > unsigned char tty_get_byte_size(unsigned int cflag, bool account_flags)
> > {
> >         unsigned char bits;
> >
> >         /* byte size and parity */

nit: byte size, the parity bit is tested later.

> >         switch (cflag & CSIZE) {
> >         case CS5:
> >                 bits = 5;
> >                 break;
> >         case CS6:
> >                 bits = 6;
> >                 break;
> >         case CS7:
> >                 bits = 7;
> >                 break;
> >         case CS8:
> >         default:
> >                 bits = 8;
> >                 break;
> >         }
> >
> >         if (account_flags) {
>
>
>
> if (!account_flags)
>   return bits;
>
> ?

<shrug> six vs half dozen: 2 level indentation vs early return.

I don't think 2 level indentation is too many.

> >                 bits += 2; /* start/stop bits */
> >
> >                 if (cflag & CSTOPB)
> >                         bits++;
> >
> >                 if (cflag & PARENB)
> >                         bits++;
> >         }

Maybe comment each test as CSTOPB and PARENB aren't completely obvious.