285 Commits
Author SHA1 Message Date
Jiri Slaby (SUSE)andGreg Kroah-Hartman 2fc541e525 tty: n_tty: use guard()s
Use guards in the n_tty code. This improves readability, makes error
handling easier, and marks locked portions of code explicit. All that
while being sure the lock is unlocked.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://patch.msgid.link/20251119100140.830761-3-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-11-21 18:30:40 +01:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman f812af3642 tty: n_tty: move more_to_be_read to the end of n_tty_read()
n_tty_read() contains "we need more data" handling deep in that
function. And there is also a label (more_to_be_read) as we handle this
situation from two places.

It makes more sense to have all "return"s accumulated at the end of
functions. And "goto" from multiple places there. Therefore, do this
with the "more_to_be_read" label in n_tty_read().

After this and the previous changes, n_tty_read() is now much more
easier to follow.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-12-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:51 -07:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 40315ce580 tty: n_tty: extract n_tty_wait_for_input()
n_tty_read() is a very long function doing too much of different stuff.
Extract the "wait for input" to a separate function:
n_tty_wait_for_input(). It returns an error (< 0), no input (0), or has
potential input (1).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-11-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:51 -07:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman aa1ebc9cff tty: n_tty: extract n_tty_continue_cookie() from n_tty_read()
n_tty_read() is a very long function doing too much of different stuff.
Extract the "cookie" (continuation read) handling to a separate
function: n_tty_continue_cookie().

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-10-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:50 -07:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 67e781f568 tty: n_tty: drop n_tty_trace()
This n_tty_trace() is an always disabled debugging macro. It comes from
commit 32f13521ca ("n_tty: Line copy to user buffer in canonical
mode").

Drop it as it is dead for over a decade.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-9-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:50 -07:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman e287d64605 tty: n_tty: clean up process_output_block()
* Use guard(mutex), which results in:
  - the function can return directly when "space == 0".
  - "i" can now be "unsigned" as it is no longer abused to hold a retval
    from tty->ops->write(). Note the compared-to "nr" is already
    "unsigned".
* The end label is now dubbed "do_write" as that is what happens there.
  Unlike the uncertain "break_out" name.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-8-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:50 -07:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman fdfa49a8c9 tty: n_tty: simplify process_output()
Using guard(mutex), the function can be written in a much more efficient
way.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-7-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:50 -07:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman d97aa06667 tty: n_tty: use uint for space returned by tty_write_room()
tty_write_room() returns an "unsigned int". So in case some insane
driver (like my tty test driver) returns (legitimate) UINT_MAX from its
tty_operations::write_room(), n_tty is confused on several places.

For example, in process_output_block(), the result of tty_write_room()
is stored into (signed) "int". So this UINT_MAX suddenly becomes -1. And
that is extended to ssize_t and returned from process_output_block().
This causes a write() to such a node to receive -EPERM (which is -1).

Fix that by using proper "unsigned int" and proper "== 0" test. And
return 0 constant directly in that "if", so that it is immediately clear
what is returned ("space" equals to 0 at that point).

Similarly for process_output() and __process_echoes().

Note this does not fix any in-tree driver as of now.

If you want "Fixes: something", it would be commit 03b3b1a240 ("tty:
make tty_operations::write_room return uint"). I intentionally do not
mark this patch by a real tag below.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-6-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:50 -07:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman d2e38e713b tty: move N_TTY_BUF_SIZE to n_tty
"N_TTY_BUF_SIZE" is private to n_tty and shall not be exposed to the
world. Definitely not in tty.h somewhere in the middle of "struct
tty_struct".

This is a remnant of moving "read_flags" to "struct n_tty_data" in
commit 3fe780b379 ("TTY: move ldisc data from tty_struct: bitmaps").
But some cleanup was needed first (in previous patches).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20250317070046.24386-5-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2025-03-20 08:00:50 -07:00
Ilpo JärvinenandGreg Kroah-Hartman b19ab7ee2c tty: n_tty: Fix buffer offsets when lookahead is used
When lookahead has "consumed" some characters (la_count > 0),
n_tty_receive_buf_standard() and n_tty_receive_buf_closing() for
characters beyond the la_count are given wrong cp/fp offsets which
leads to duplicating and losing some characters.

If la_count > 0, correct buffer pointers and make count consistent too
(the latter is not strictly necessary to fix the issue but seems more
logical to adjust all variables immediately to keep state consistent).

Reported-by: Vadym Krevs <vkrevs@yahoo.com>
Fixes: 6bb6fa6908 ("tty: Implement lookahead to process XON/XOFF timely")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218834
Tested-by: Vadym Krevs <vkrevs@yahoo.com>
Cc: stable@vger.kernel.org
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20240514140429.12087-1-ilpo.jarvinen@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2024-06-04 14:07:27 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 5b4f9cf3cc tty: invert return values of tty_{,un}throttle_safe()
If tty_{,un}throttle_safe() returned true on success (similar to
*_trylock()), it would make the conditions in callers more obvious. So
perform the switch to these inverted values (and fix the callers).

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230919085156.1578-8-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-10-03 14:31:15 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 043c8a7c01 tty: n_tty: use do-while in n_tty_check_{,un}throttle()
This change gets rid of the complicated exit from the loops. It can be
done much easier using do-while loops.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230919085156.1578-6-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-10-03 14:31:15 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman c2b0fb9f69 tty: n_tty: invert the condition in copy_from_read_buf()
Make "no numbers available" a fast quit from the function. And do the
heavy work outside the 'if'. This makes the code more understandable and
conforming to the common kernel coding style.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Link: https://lore.kernel.org/r/20230919085156.1578-5-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-10-03 14:31:15 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 72369f2d49 tty: n_tty: use min3() in copy_from_read_buf()
n is a minimum of:
* available chars in the ring buffer
* available chars in the ring buffer till the end of the ring buffer
* requested number (*nr)

We can use min3() for that instead of two min()s.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230919085156.1578-4-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-10-03 14:31:15 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 1e619477a9 tty: n_tty: rename and retype 'retval' in n_tty_ioctl()
The value stored to the current 'retval' is number of characters. It is
both obtained and put to user as unsigned. So make its type unsigned.
And provided it's not a "return value" per se, rename it to 'num'.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230919085156.1578-3-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-10-03 14:31:15 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 4a2ad26613 tty: n_tty: use 'retval' instead of 'c'
In n_tty_read(), there is a separate int variable 'c' and is used only
to hold an int value returned from job_control(). There is also a
'retval' variable typed ssize_t. So drop this single occurrence of 'c'
and reuse 'retval' which is used on all other places to hold the value
returned from n_tty_read().

Note that 'retval' needs not be initialized now. Drop that.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230919085156.1578-2-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-10-03 14:31:15 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman a84853c595 tty: n_tty: deduplicate copy code in n_tty_receive_buf_real_raw()
The code is duplicated to perform the copy twice -- to handle buffer
wrap-around. Instead of the duplication, roll this into the loop.

(And add some blank lines around to have the code a bit more readable.)

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-15-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 2aa91851ff tty: n_tty: extract ECHO_OP processing to a separate function
__process_echoes() contains ECHO_OPs processing. It is stuffed in a
while loop and the whole function is barely readable. Separate it to a
new function: n_tty_process_echo_ops().

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-14-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman e30364c708 tty: n_tty: unify counts to size_t
Some count types are already 'size_t' for a long time. Some were
switched to 'size_t' recently. Unify the rest with those now.

This allows for some min_t()s to become min()s. And make one min()
an explicit min_t() as we are comparing signed 'room' to unsigned
'count'.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-13-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman b9b96b2089 tty: n_tty: use u8 for chars and flags
Unify with the tty layer and use u8 for both chars and flags.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-12-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman d88c3c2675 tty: n_tty: simplify chars_in_buffer()
The 'if' in chars_in_buffer() is misleadingly inverted. And since the
only difference is the head used for computation, cache the head using
ternary operator. And use that in return directly.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-11-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 046b44ab0f tty: n_tty: remove unsigned char casts from character constants
We compile with -funsigned-char, so all character constants are already
unsigned chars. Therefore, remove superfluous casts.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-10-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 008304079d tty: n_tty: move newline handling to a separate function
Currently, n_tty handles the newline in a label in
n_tty_receive_char_canon(). That is invoked from two more places. Split
this code to a separate function and avoid the label in this case.

This makes the code flow more understandable.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-9-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 102dc8aac8 tty: n_tty: move canon handling to a separate function
n_tty_receive_char_special() is already complicated enough. Split the
canon handling to a separate function: n_tty_receive_char_canon().

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-8-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00
Jiri Slaby (SUSE)andGreg Kroah-Hartman 819287f0f3 tty: n_tty: use MASK() for masking out size bits
In n_tty, there is already a macro to mask out top bits from ring buffer
counters. It is MASK() added some time ago. So use it more in the code
to make it more readable.

Signed-off-by: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Link: https://lore.kernel.org/r/20230827074147.2287-7-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-08-27 11:46:52 +02:00