[12/15] drivers: net: slip: remove SLIP_MAGIC
Commit Message
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>
---
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(-)
Comments
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 */
On Thu, Oct 27, 2022 at 03:11:56PM +0200, Oliver Hartkopp wrote:
> 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 ;-)
According to get_maintainer.pl,
(which I just shoved into Cc: indiscriminately,
lacking any knowledge to the contrary),
you get picked up for slip.{c,h} as commit_signer:2/4=50% and 1/2, resp.
As does Jiri, so on that front we're good at least.
Best,
On 10/27/22 05:43, наб wrote:
> 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
>
Same reply as [1].
[1]: https://lore.kernel.org/linux-doc/9386b19f-dd99-3601-9e87-3056100dfe53@gmail.com/
@@ -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``
===================== ================ ======================== ==========================================
@@ -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``
===================== ================ ======================== ==========================================
@@ -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``
===================== ================ ======================== ==========================================
@@ -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``
===================== ================ ======================== ==========================================
@@ -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) {
@@ -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 */