Re: [PATCH 3/5] tty: n_hdlc: remove HDLC_MAGIC

Jiri Slaby
2022-09-16T06:51:59Z

On 16. 09. 22, 3:55, наб wrote:
> According to Greg, in the context of magic numbers as defined in
> magic-number.rst, "the tty layer should not need this and I'll gladly
> take patches"
>
> Ref: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Acked-by: Jiri Slaby <jirislaby@kernel.org>

> ---
> Documentation/process/magic-number.rst | 1 -
> .../it_IT/process/magic-number.rst | 1 -
> .../zh_CN/process/magic-number.rst | 1 -
> .../zh_TW/process/magic-number.rst | 1 -
> drivers/tty/n_hdlc.c | 27 -------------------
> 5 files changed, 31 deletions(-)
>
> diff --git a/Documentation/process/magic-number.rst b/Documentation/process/magic-number.rst
> index d47799ba0ca4..fdaa3e4b1953 100644
> --- a/Documentation/process/magic-number.rst
> +++ b/Documentation/process/magic-number.rst
> @@ -71,7 +71,6 @@ Magic Name Number Structure File
> PG_MAGIC 'P' pg_{read,write}_hdr ``include/linux/pg.h``
> CMAGIC 0x0111 user ``include/linux/a.out.h``
> MKISS_DRIVER_MAGIC 0x04bf mkiss_channel ``drivers/net/mkiss.h``
> -HDLC_MAGIC 0x239e n_hdlc ``drivers/char/n_hdlc.c``
> APM_BIOS_MAGIC 0x4101 apm_user ``arch/x86/kernel/apm_32.c``
> DB_MAGIC 0x4442 fc_info ``drivers/net/iph5526_novram.c``
> DL_MAGIC 0x444d fc_info ``drivers/net/iph5526_novram.c``
> diff --git a/Documentation/translations/it_IT/process/magic-number.rst b/Documentation/translations/it_IT/process/magic-number.rst
> index 24022ab52ebb..1898f98875de 100644
> --- a/Documentation/translations/it_IT/process/magic-number.rst
> +++ b/Documentation/translations/it_IT/process/magic-number.rst
> @@ -77,7 +77,6 @@ Nome magico Numero Struttura File
> PG_MAGIC 'P' pg_{read,write}_hdr ``include/linux/pg.h``
> CMAGIC 0x0111 user ``include/linux/a.out.h``
> MKISS_DRIVER_MAGIC 0x04bf mkiss_channel ``drivers/net/mkiss.h``
> -HDLC_MAGIC 0x239e n_hdlc ``drivers/char/n_hdlc.c``
> APM_BIOS_MAGIC 0x4101 apm_user ``arch/x86/kernel/apm_32.c``
> DB_MAGIC 0x4442 fc_info ``drivers/net/iph5526_novram.c``
> DL_MAGIC 0x444d fc_info ``drivers/net/iph5526_novram.c``
> diff --git a/Documentation/translations/zh_CN/process/magic-number.rst b/Documentation/translations/zh_CN/process/magic-number.rst
> index 811804996283..911cdaeaf698 100644
> --- a/Documentation/translations/zh_CN/process/magic-number.rst
> +++ b/Documentation/translations/zh_CN/process/magic-number.rst
> @@ -60,7 +60,6 @@ Linux 魔术数
> PG_MAGIC 'P' pg_{read,write}_hdr ``include/linux/pg.h``
> CMAGIC 0x0111 user ``include/linux/a.out.h``
> MKISS_DRIVER_MAGIC 0x04bf mkiss_channel ``drivers/net/mkiss.h``
> -HDLC_MAGIC 0x239e n_hdlc ``drivers/char/n_hdlc.c``
> APM_BIOS_MAGIC 0x4101 apm_user ``arch/x86/kernel/apm_32.c``
> DB_MAGIC 0x4442 fc_info ``drivers/net/iph5526_novram.c``
> DL_MAGIC 0x444d fc_info ``drivers/net/iph5526_novram.c``
> diff --git a/Documentation/translations/zh_TW/process/magic-number.rst b/Documentation/translations/zh_TW/process/magic-number.rst
> index 8e37e00590f5..ac87f188235f 100644
> --- a/Documentation/translations/zh_TW/process/magic-number.rst
> +++ b/Documentation/translations/zh_TW/process/magic-number.rst
> @@ -63,7 +63,6 @@ Linux 魔術數
> PG_MAGIC 'P' pg_{read,write}_hdr ``include/linux/pg.h``
> CMAGIC 0x0111 user ``include/linux/a.out.h``
> MKISS_DRIVER_MAGIC 0x04bf mkiss_channel ``drivers/net/mkiss.h``
> -HDLC_MAGIC 0x239e n_hdlc ``drivers/char/n_hdlc.c``
> APM_BIOS_MAGIC 0x4101 apm_user ``arch/x86/kernel/apm_32.c``
> DB_MAGIC 0x4442 fc_info ``drivers/net/iph5526_novram.c``
> DL_MAGIC 0x444d fc_info ``drivers/net/iph5526_novram.c``
> diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
> index 94c1ec2dd754..b5aa27051119 100644
> --- a/drivers/tty/n_hdlc.c
> +++ b/drivers/tty/n_hdlc.c
> @@ -76,8 +76,6 @@
> * OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> -#define HDLC_MAGIC 0x239e
> -
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> @@ -124,7 +122,6 @@ struct n_hdlc_buf_list {
>
> /**
> * struct n_hdlc - per device instance data structure
> - * @magic: magic value for structure
> * @tbusy: reentrancy flag for tx wakeup code
> * @woke_up: tx wakeup needs to be run again as it was called while @tbusy
> * @tx_buf_list: list of pending transmit frame buffers
> @@ -133,7 +130,6 @@ struct n_hdlc_buf_list {
> * @rx_free_buf_list: list unused received frame buffers
> */
> struct n_hdlc {
> - int magic;
> bool tbusy;
> bool woke_up;
> struct n_hdlc_buf_list tx_buf_list;
> @@ -200,10 +196,6 @@ static void n_hdlc_tty_close(struct tty_struct *tty)
> {
> struct n_hdlc *n_hdlc = tty->disc_data;
>
> - if (n_hdlc->magic != HDLC_MAGIC) {
> - pr_warn("n_hdlc: trying to close unopened tty!\n");
> - return;
> - }
> #if defined(TTY_NO_WRITE_SPLIT)
> clear_bit(TTY_NO_WRITE_SPLIT, &tty->flags);
> #endif
> @@ -386,12 +378,6 @@ static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
>
> pr_debug("%s() called count=%d\n", __func__, count);
>
> - /* verify line is using HDLC discipline */
> - if (n_hdlc->magic != HDLC_MAGIC) {
> - pr_err("line not using HDLC discipline\n");
> - return;
> - }
> -
> if (count > maxframe) {
> pr_debug("rx count>maxframesize, data discarded\n");
> return;
> @@ -542,9 +528,6 @@ static ssize_t n_hdlc_tty_write(struct tty_struct *tty, struct file *file,
>
> pr_debug("%s() called count=%zd\n", __func__, count);
>
> - if (n_hdlc->magic != HDLC_MAGIC)
> - return -EIO;
> -
> /* verify frame size */
> if (count > maxframe) {
> pr_debug("%s: truncating user packet from %zu to %d\n",
> @@ -609,10 +592,6 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
>
> pr_debug("%s() called %d\n", __func__, cmd);
>
> - /* Verify the status of the device */
> - if (n_hdlc->magic != HDLC_MAGIC)
> - return -EBADF;
> -
> switch (cmd) {
> case FIONREAD:
> /* report count of read data available */
> @@ -673,9 +652,6 @@ static __poll_t n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
> struct n_hdlc *n_hdlc = tty->disc_data;
> __poll_t mask = 0;
>
> - if (n_hdlc->magic != HDLC_MAGIC)
> - return 0;
> -
> /*
> * queue the current process into any wait queue that may awaken in the
> * future (read and write)
> @@ -739,9 +715,6 @@ static struct n_hdlc *n_hdlc_alloc(void)
> n_hdlc_alloc_buf(&n_hdlc->rx_free_buf_list, DEFAULT_RX_BUF_COUNT, "rx");
> n_hdlc_alloc_buf(&n_hdlc->tx_free_buf_list, DEFAULT_TX_BUF_COUNT, "tx");
>
> - /* Initialize the control block */
> - n_hdlc->magic = HDLC_MAGIC;
> -
> return n_hdlc;
>
> } /* end of n_hdlc_alloc() */

--
js
suse labs