Re: [PATCH 12/15] drivers: net: slip: remove SLIP_MAGIC

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hi,

I'm not sure why I'm in 'To' here as I'm definitely not the official 
maintainer of slip.

But it looks like there is no real maintainer anyway but maybe Jiri ;-)

On 27.10.22 00:43, наб 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"
> 
> We have largely moved away from this approach,
> and we have better debugging instrumentation nowadays: kill it
> 
> Additionally, all SLIP_MAGIC checks just early-exit instead
> of noting the bug, so they're detrimental, if anything
> 
> Ref: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>


Many thanks!
Oliver

> ---
>   Documentation/process/magic-number.rst                |  1 -
>   .../translations/it_IT/process/magic-number.rst       |  1 -
>   .../translations/zh_CN/process/magic-number.rst       |  1 -
>   .../translations/zh_TW/process/magic-number.rst       |  1 -
>   drivers/net/slip/slip.c                               | 11 +++++------
>   drivers/net/slip/slip.h                               |  4 ----
>   6 files changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/process/magic-number.rst b/Documentation/process/magic-number.rst
> index 3b3e607e1cbc..e59c707ec785 100644
> --- a/Documentation/process/magic-number.rst
> +++ b/Documentation/process/magic-number.rst
> @@ -69,6 +69,5 @@ Changelog::
>   Magic Name            Number           Structure                File
>   ===================== ================ ======================== ==========================================
>   FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
> -SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
>   CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
>   ===================== ================ ======================== ==========================================
> diff --git a/Documentation/translations/it_IT/process/magic-number.rst b/Documentation/translations/it_IT/process/magic-number.rst
> index e8c659b6a743..37a539867b6f 100644
> --- a/Documentation/translations/it_IT/process/magic-number.rst
> +++ b/Documentation/translations/it_IT/process/magic-number.rst
> @@ -75,6 +75,5 @@ Registro dei cambiamenti::
>   Nome magico           Numero           Struttura                File
>   ===================== ================ ======================== ==========================================
>   FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
> -SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
>   CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
>   ===================== ================ ======================== ==========================================
> diff --git a/Documentation/translations/zh_CN/process/magic-number.rst b/Documentation/translations/zh_CN/process/magic-number.rst
> index 2105af32187c..8a3a3e872c52 100644
> --- a/Documentation/translations/zh_CN/process/magic-number.rst
> +++ b/Documentation/translations/zh_CN/process/magic-number.rst
> @@ -58,6 +58,5 @@ Linux 魔术数
>   魔术数名              数字             结构                     文件
>   ===================== ================ ======================== ==========================================
>   FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
> -SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
>   CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
>   ===================== ================ ======================== ==========================================
> diff --git a/Documentation/translations/zh_TW/process/magic-number.rst b/Documentation/translations/zh_TW/process/magic-number.rst
> index 793a0ae9fb7c..7ace7834f7f9 100644
> --- a/Documentation/translations/zh_TW/process/magic-number.rst
> +++ b/Documentation/translations/zh_TW/process/magic-number.rst
> @@ -61,6 +61,5 @@ Linux 魔術數
>   魔術數名              數字             結構                     文件
>   ===================== ================ ======================== ==========================================
>   FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
> -SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
>   CCB_MAGIC             0xf2691ad2       ccb                      ``drivers/scsi/ncr53c8xx.c``
>   ===================== ================ ======================== ==========================================
> diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
> index 6865d32270e5..95f5c79772e7 100644
> --- a/drivers/net/slip/slip.c
> +++ b/drivers/net/slip/slip.c
> @@ -426,7 +426,7 @@ static void slip_transmit(struct work_struct *work)
>   
>   	spin_lock_bh(&sl->lock);
>   	/* First make sure we're connected. */
> -	if (!sl->tty || sl->magic != SLIP_MAGIC || !netif_running(sl->dev)) {
> +	if (!sl->tty || !netif_running(sl->dev)) {
>   		spin_unlock_bh(&sl->lock);
>   		return;
>   	}
> @@ -690,7 +690,7 @@ static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
>   {
>   	struct slip *sl = tty->disc_data;
>   
> -	if (!sl || sl->magic != SLIP_MAGIC || !netif_running(sl->dev))
> +	if (!sl || !netif_running(sl->dev))
>   		return;
>   
>   	/* Read the characters out of the buffer */
> @@ -761,7 +761,6 @@ static struct slip *sl_alloc(void)
>   	sl = netdev_priv(dev);
>   
>   	/* Initialize channel control data */
> -	sl->magic       = SLIP_MAGIC;
>   	sl->dev	      	= dev;
>   	spin_lock_init(&sl->lock);
>   	INIT_WORK(&sl->tx_work, slip_transmit);
> @@ -809,7 +808,7 @@ static int slip_open(struct tty_struct *tty)
>   
>   	err = -EEXIST;
>   	/* First make sure we're not already connected. */
> -	if (sl && sl->magic == SLIP_MAGIC)
> +	if (sl)
>   		goto err_exit;
>   
>   	/* OK.  Find a free SLIP channel to use. */
> @@ -886,7 +885,7 @@ static void slip_close(struct tty_struct *tty)
>   	struct slip *sl = tty->disc_data;
>   
>   	/* First make sure we're connected. */
> -	if (!sl || sl->magic != SLIP_MAGIC || sl->tty != tty)
> +	if (!sl || sl->tty != tty)
>   		return;
>   
>   	spin_lock_bh(&sl->lock);
> @@ -1080,7 +1079,7 @@ static int slip_ioctl(struct tty_struct *tty, unsigned int cmd,
>   	int __user *p = (int __user *)arg;
>   
>   	/* First make sure we're connected. */
> -	if (!sl || sl->magic != SLIP_MAGIC)
> +	if (!sl)
>   		return -EINVAL;
>   
>   	switch (cmd) {
> diff --git a/drivers/net/slip/slip.h b/drivers/net/slip/slip.h
> index 3d7f88b330c1..d7dbedd27669 100644
> --- a/drivers/net/slip/slip.h
> +++ b/drivers/net/slip/slip.h
> @@ -50,8 +50,6 @@
>   
>   
>   struct slip {
> -  int			magic;
> -
>     /* Various fields. */
>     struct tty_struct	*tty;		/* ptr to TTY structure		*/
>     struct net_device	*dev;		/* easy for intr handling	*/
> @@ -100,6 +98,4 @@ struct slip {
>   #endif
>   };
>   
> -#define SLIP_MAGIC 0x5302
> -
>   #endif	/* _LINUX_SLIP.H */