linux-kernel - Re: [PATCH 34/35] tty: make tty_get_byte

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <e5ca055cf46147537748c7186a6a031ab7663ad8.camel@perches.com>
Date:   Thu, 06 May 2021 01:35:57 -0700
From:   Joe Perches <joe@...ches.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Jiri Slaby <jslaby@...e.cz>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 34/35] tty: make tty_get_byte_size available

On Thu, 2021-05-06 at 11:24 +0300, Andy Shevchenko wrote:
> On Thursday, May 6, 2021, Joe Perches <joe@...ches.com> 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.