[01/15] hamradio: baycom: remove BAYCOM_MAGIC

Message ID 9a453437b5c3b4b1887c1bd84455b0cc3d1c40b2.1666822928.git.nabijaczleweli@nabijaczleweli.xyz
State New
Headers
Series [01/15] hamradio: baycom: remove BAYCOM_MAGIC |

Commit Message

Ahelenia Ziemiańska Oct. 26, 2022, 10:42 p.m. UTC
  Since defanging in v2.6.12-rc1 it's set exactly once per port on probe
and checked exactly once per port on unload: it's useless. Kill it.

Notably, magic-number.rst has never had the right value for it with the
new-in-2.1.105 network-based driver

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Ref: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/
---
 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/hamradio/baycom_epp.c                 | 15 ++-------------
 5 files changed, 2 insertions(+), 17 deletions(-)
  

Comments

Jakub Kicinski Oct. 27, 2022, 1:37 a.m. UTC | #1
On Thu, 27 Oct 2022 00:42:37 +0200 наб wrote:
> Since defanging in v2.6.12-rc1 it's set exactly once per port on probe
> and checked exactly once per port on unload: it's useless. Kill it.
> 
> Notably, magic-number.rst has never had the right value for it with the
> new-in-2.1.105 network-based driver
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Interesting name :S Just to be clear full legal names are required.
Not saying that's not your name, just saying I haven't heard it before.

Plus the From line must be the same, legal name.

> Ref: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/

Link: is the tag we use.

Otherwise looks fine.
  
Bagas Sanjaya Oct. 28, 2022, 1:54 a.m. UTC | #2
On 10/27/22 08:37, Jakub Kicinski wrote:
> On Thu, 27 Oct 2022 00:42:37 +0200 наб wrote:
>> Since defanging in v2.6.12-rc1 it's set exactly once per port on probe
>> and checked exactly once per port on unload: it's useless. Kill it.
>>
>> Notably, magic-number.rst has never had the right value for it with the
>> new-in-2.1.105 network-based driver
>>
>> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> 
> Interesting name :S Just to be clear full legal names are required.
> Not saying that's not your name, just saying I haven't heard it before.
> 
> Plus the From line must be the same, legal name.
> 

You mean From header (email).

In this case, she forgot to add From: tag before the patch description
(which could be easily added with --from option to git-format-patch(1)).

Thanks.
  
Bagas Sanjaya Oct. 28, 2022, 1:13 p.m. UTC | #3
On 10/27/22 05:42, наб wrote:
> Since defanging in v2.6.12-rc1 it's set exactly once per port on probe
> and checked exactly once per port on unload: it's useless. Kill it.
> 

What do you mean by defanging in that release?

Also, s/Kill it/Remove BAYCOM_MAGIC from magic numbers table/ (your
wording is kinda mature).

> Notably, magic-number.rst has never had the right value for it with the
> new-in-2.1.105 network-based driver
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> Ref: https://lore.kernel.org/linux-doc/YyMlovoskUcHLEb7@kroah.com/

Use Link: tag instead.

Thanks.
  
Dan Carpenter Oct. 28, 2022, 1:43 p.m. UTC | #4
On Fri, Oct 28, 2022 at 08:13:49PM +0700, Bagas Sanjaya wrote:
> On 10/27/22 05:42, наб wrote:
> > Since defanging in v2.6.12-rc1 it's set exactly once per port on probe
> > and checked exactly once per port on unload: it's useless. Kill it.
> > 
> 
> What do you mean by defanging in that release?

I was also curious about this, but I accidentally sent my question
privately instead of to the list.

> 
> Also, s/Kill it/Remove BAYCOM_MAGIC from magic numbers table/ (your
> wording is kinda mature).
> 

The kernel has almost 13 thousand kills...

$ git grep -i kill | wc -l
12975
$

It's fine.

regards,
dan carpenter
  
Bagas Sanjaya Oct. 28, 2022, 1:50 p.m. UTC | #5
On 10/28/22 20:43, Dan Carpenter wrote:
>>
>> Also, s/Kill it/Remove BAYCOM_MAGIC from magic numbers table/ (your
>> wording is kinda mature).
>>
> 
> The kernel has almost 13 thousand kills...
> 
> $ git grep -i kill | wc -l
> 12975
> $
> 
> It's fine.
> 

The word meaning depends on context. In this case, the author means
removing SOME_MAGIC magic number from the table, one by one until
the magic number documentation is removed (due to historical cruft).

Thanks.
  
Dan Carpenter Oct. 28, 2022, 2:07 p.m. UTC | #6
On Fri, Oct 28, 2022 at 08:50:59PM +0700, Bagas Sanjaya wrote:
> On 10/28/22 20:43, Dan Carpenter wrote:
> >>
> >> Also, s/Kill it/Remove BAYCOM_MAGIC from magic numbers table/ (your
> >> wording is kinda mature).
> >>
> > 
> > The kernel has almost 13 thousand kills...
> > 
> > $ git grep -i kill | wc -l
> > 12975
> > $
> > 
> > It's fine.
> > 
> 
> The word meaning depends on context. In this case, the author means
> removing SOME_MAGIC magic number from the table, one by one until
> the magic number documentation is removed (due to historical cruft).

Kernel devs are naturally blood thirsty people...

$ git log --after=2022-01-01 | grep -w kill | wc -l
207

When people talk about killing stuff they mostly mean deleting code.
Look at this sample of the very first kills from January.  Seven out
of ten times this is what they meant.

$ git log --after=2022-01-01 | grep -w kill | head -n 10
    useless now, kill it.
    net: ipa: kill ipa_table_valid()
    net: ipa: kill two constant symbols
      io_uring: kill hot path fixed file bitmap debug checks
                   newattrs.ia_valid = ATTR_FORCE | kill;
                newattrs.ia_valid = ATTR_FORCE | kill;
                        newattrs.ia_valid = attr_force | kill;
    kill it off before it ends up in a release. It was just part of the
    io_uring: kill hot path fixed file bitmap debug checks
      block: kill deprecated BUG_ON() in the flush handling
$

regards,
dan carpenter
  
Ahelenia Ziemiańska Oct. 28, 2022, 9:43 p.m. UTC | #7
On Fri, Oct 28, 2022 at 08:13:49PM +0700, Bagas Sanjaya wrote:
> On 10/27/22 05:42, наб wrote:
> > Since defanging in v2.6.12-rc1 it's set exactly once per port on probe
> > and checked exactly once per port on unload: it's useless. Kill it.
> > 
> 
> What do you mean by defanging in that release?

Before v2.6.12-rc1, there were baycom_paranoia_check{,_void}() macros
(which logged at KERN_ERR and returned an error),
used before each netdev_priv(); after, this is reduced to the single
check in the cleanup we observe presently.

I've rephrased the message to make it more palatable.

Best,
  

Patch

diff --git a/Documentation/process/magic-number.rst b/Documentation/process/magic-number.rst
index 64b5948fc1d4..87353e0207c1 100644
--- a/Documentation/process/magic-number.rst
+++ b/Documentation/process/magic-number.rst
@@ -73,7 +73,6 @@  APM_BIOS_MAGIC        0x4101           apm_user                 ``arch/x86/kerne
 FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 MGSLPC_MAGIC          0x5402           mgslpc_info              ``drivers/char/pcmcia/synclink_cs.c``
-BAYCOM_MAGIC          0x19730510       baycom_state             ``drivers/net/baycom_epp.c``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.h``
 KV_MAGIC              0x5f4b565f       kernel_vars_s            ``arch/mips/include/asm/sn/klkernvars.h``
 CODA_MAGIC            0xC0DAC0DA       coda_file_info           ``fs/coda/coda_fs_i.h``
diff --git a/Documentation/translations/it_IT/process/magic-number.rst b/Documentation/translations/it_IT/process/magic-number.rst
index 02eb7eb2448e..a96712bd8a68 100644
--- a/Documentation/translations/it_IT/process/magic-number.rst
+++ b/Documentation/translations/it_IT/process/magic-number.rst
@@ -79,7 +79,6 @@  APM_BIOS_MAGIC        0x4101           apm_user                 ``arch/x86/kerne
 FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 MGSLPC_MAGIC          0x5402           mgslpc_info              ``drivers/char/pcmcia/synclink_cs.c``
-BAYCOM_MAGIC          0x19730510       baycom_state             ``drivers/net/baycom_epp.c``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.h``
 KV_MAGIC              0x5f4b565f       kernel_vars_s            ``arch/mips/include/asm/sn/klkernvars.h``
 CODA_MAGIC            0xC0DAC0DA       coda_file_info           ``fs/coda/coda_fs_i.h``
diff --git a/Documentation/translations/zh_CN/process/magic-number.rst b/Documentation/translations/zh_CN/process/magic-number.rst
index 0617ce125e12..44f3a29fce57 100644
--- a/Documentation/translations/zh_CN/process/magic-number.rst
+++ b/Documentation/translations/zh_CN/process/magic-number.rst
@@ -62,7 +62,6 @@  APM_BIOS_MAGIC        0x4101           apm_user                 ``arch/x86/kerne
 FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 MGSLPC_MAGIC          0x5402           mgslpc_info              ``drivers/char/pcmcia/synclink_cs.c``
-BAYCOM_MAGIC          0x19730510       baycom_state             ``drivers/net/baycom_epp.c``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.h``
 KV_MAGIC              0x5f4b565f       kernel_vars_s            ``arch/mips/include/asm/sn/klkernvars.h``
 CODA_MAGIC            0xC0DAC0DA       coda_file_info           ``fs/coda/coda_fs_i.h``
diff --git a/Documentation/translations/zh_TW/process/magic-number.rst b/Documentation/translations/zh_TW/process/magic-number.rst
index f3f7082e17c6..0fde3183e82a 100644
--- a/Documentation/translations/zh_TW/process/magic-number.rst
+++ b/Documentation/translations/zh_TW/process/magic-number.rst
@@ -65,7 +65,6 @@  APM_BIOS_MAGIC        0x4101           apm_user                 ``arch/x86/kerne
 FASYNC_MAGIC          0x4601           fasync_struct            ``include/linux/fs.h``
 SLIP_MAGIC            0x5302           slip                     ``drivers/net/slip.h``
 MGSLPC_MAGIC          0x5402           mgslpc_info              ``drivers/char/pcmcia/synclink_cs.c``
-BAYCOM_MAGIC          0x19730510       baycom_state             ``drivers/net/baycom_epp.c``
 HDLCDRV_MAGIC         0x5ac6e778       hdlcdrv_state            ``include/linux/hdlcdrv.h``
 KV_MAGIC              0x5f4b565f       kernel_vars_s            ``arch/mips/include/asm/sn/klkernvars.h``
 CODA_MAGIC            0xC0DAC0DA       coda_file_info           ``fs/coda/coda_fs_i.h``
diff --git a/drivers/net/hamradio/baycom_epp.c b/drivers/net/hamradio/baycom_epp.c
index 791b4a53d69f..8f018703e74d 100644
--- a/drivers/net/hamradio/baycom_epp.c
+++ b/drivers/net/hamradio/baycom_epp.c
@@ -45,13 +45,9 @@ 
 /* --------------------------------------------------------------------- */
 
 #define BAYCOM_DEBUG
-#define BAYCOM_MAGIC 19730510
 
 /* --------------------------------------------------------------------- */
 
-static const char paranoia_str[] = KERN_ERR 
-	"baycom_epp: bad magic number for hdlcdrv_state struct in routine %s\n";
-
 static const char bc_drvname[] = "baycom_epp";
 static const char bc_drvinfo[] = KERN_INFO "baycom_epp: (C) 1998-2000 Thomas Sailer, HB9JNX/AE4WA\n"
 "baycom_epp: version 0.7\n";
@@ -152,8 +148,6 @@  static struct net_device *baycom_device[NR_PORTS];
  */
 
 struct baycom_state {
-	int magic;
-
         struct pardevice *pdev;
 	struct net_device *dev;
 	unsigned int work_running;
@@ -1210,7 +1204,6 @@  static void __init baycom_epp_dev_setup(struct net_device *dev)
 	 * initialize part of the baycom_state struct
 	 */
 	bc->dev = dev;
-	bc->magic = BAYCOM_MAGIC;
 	bc->cfg.fclk = 19666600;
 	bc->cfg.bps = 9600;
 	/*
@@ -1279,12 +1272,8 @@  static void __exit cleanup_baycomepp(void)
 		struct net_device *dev = baycom_device[i];
 
 		if (dev) {
-			struct baycom_state *bc = netdev_priv(dev);
-			if (bc->magic == BAYCOM_MAGIC) {
-				unregister_netdev(dev);
-				free_netdev(dev);
-			} else
-				printk(paranoia_str, "cleanup_module");
+			unregister_netdev(dev);
+			free_netdev(dev);
 		}
 	}
 	parport_unregister_driver(&baycom_epp_par_driver);