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) <[email protected]>
Suggested-by: Ilpo Järvinen <[email protected]>
---
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
@@ -393,8 +393,10 @@ extern const struct class tty_class;
* tty_kref_get - get a tty reference
* @tty: tty device
*
- * Returns: a new reference to a tty object. The caller must hold sufficient
- * locks/counts to ensure that their existing reference cannot go away
+ * Returns: a new reference to a tty object
+ *
+ * Locking: The caller must hold sufficient locks/counts to ensure that their
+ * existing reference cannot go away.
*/
static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
{
@@ -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.
*/
static inline speed_t tty_get_baud_rate(struct tty_struct *tty)
{
--
2.42.1
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) <[email protected]>
> Suggested-by: Ilpo Järvinen <[email protected]>
> ---
> 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
> @@ -393,8 +393,10 @@ extern const struct class tty_class;
> * tty_kref_get - get a tty reference
> * @tty: tty device
> *
> - * Returns: a new reference to a tty object. The caller must hold sufficient
> - * locks/counts to ensure that their existing reference cannot go away
> + * Returns: a new reference to a tty object
> + *
> + * Locking: The caller must hold sufficient locks/counts to ensure that their
> + * existing reference cannot go away.
Just noting this is a bit vaguely worded (but so is the original).
> */
> static inline struct tty_struct *tty_kref_get(struct tty_struct *tty)
> {
> @@ -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.
Other than that,
Reviewed-by: Ilpo Järvinen <[email protected]>
--
i.
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) <[email protected]>
>> Suggested-by: Ilpo Järvinen <[email protected]>
>> ---
>> 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.
thanks,
--
js
suse labs
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) <[email protected]>
>>> Suggested-by: Ilpo Järvinen <[email protected]>
>>> ---
>>> 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 <[email protected]>
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.
> thanks,
--
js
suse labs
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) <[email protected]>
> > > > Suggested-by: Ilpo Järvinen <[email protected]>
> > > > ---
> > > > 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 <[email protected]>
> 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.