linux-kernel - Re: [PATCH 04/17] tty: move locking docs out of Returns for functions in tty.h

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <46ef4b89-6a6b-ddb1-fec8-eb9749e9b0f3@linux.intel.com>
Date:   Wed, 22 Nov 2023 12:26:55 +0200 (EET)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     Jiri Slaby <jirislaby@...nel.org>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-serial <linux-serial@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 04/17] tty: move locking docs out of Returns for functions
 in tty.h

On Wed, 22 Nov 2023, Jiri Slaby wrote:

> On 22. 11. 23, 7:45, Jiri Slaby wrote:
> > On 21. 11. 23, 10:48, Ilpo Järvinen wrote:
> > > On Tue, 21 Nov 2023, Jiri Slaby (SUSE) wrote:
> > > 
> > > > Both tty_kref_get() and tty_get_baud_rate() note about locking in their
> > > > Return kernel-doc clause. Extract this info into a separate "Locking"
> > > > paragraph -- the same as we do for other tty functions.
> > > > 
> > > > Signed-off-by: Jiri Slaby (SUSE) <jirislaby@...nel.org>
> > > > Suggested-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> > > > ---
> > > >   include/linux/tty.h | 12 +++++++-----
> > > >   1 file changed, 7 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > > > index 4b6340ac2af2..7625fc98fef3 100644
> > > > --- a/include/linux/tty.h
> > > > +++ b/include/linux/tty.h
> > ...
> > > > @@ -436,10 +438,10 @@ void tty_encode_baud_rate(struct tty_struct *tty,
> > > > speed_t ibaud,
> > > >    * tty_get_baud_rate - get tty bit rates
> > > >    * @tty: tty to query
> > > >    *
> > > > - * Returns: the baud rate as an integer for this terminal. The termios
> > > > lock
> > > > - * must be held by the caller and the terminal bit flags may be
> > > > updated.
> > > > + * Returns: the baud rate as an integer for this terminal
> > > >    *
> > > > - * Locking: none
> > > > + * Locking: The termios lock must be held by the caller and the
> > > > terminal bit
> > > > + * flags may be updated.
> > > 
> > > I don't think the second part about the flags really belongs here, I'd
> > > keep it the description.
> > 
> > Any idea what does the part says in fact? I had not been thinking about the
> > content and now I don't understand it.
> 
> Maybe before:
> commit 6865ff222ccab371c04afce17aec1f7d70b17dbc
> Author: Jiri Slaby <jirislaby@...nel.org>
> Date:   Thu Mar 7 13:12:27 2013 +0100
> 
>     TTY: do not warn about setting speed via SPD_*
> 
> 
> tty->warned was "the terminal bit".
> 
> Let's drop that part. And we can make tty const there too.

Good point. 

The commit you point to is probably unrelated though but thanks anyway 
because it gave me this idea which I think is the correct reference: I 
removed the ->c_cflag alteringin 87888fb9ac0c ("tty: Remove baudrate dead 
code & make ktermios params const"). It had become deadcode anyway long 
since (I never went through the arch headers to find how long ago).

So yes, dropping the second part seems the correct way to go.

-- 
 i.