| Message ID | f5f9036f2a488886fe5a424d8143e8f2f3fdcf3f.1666822928.git.nabijaczleweli@nabijaczleweli.xyz (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [01/15] hamradio: baycom: remove BAYCOM_MAGIC | expand |
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/
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 */
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(-)