[Cocci] [PATCH] net: usb: hso.c: remove unneeded DRIVER

Discussion:

[Cocci] [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

Greg Kroah-Hartman

2017-11-22 17:12:01 UTC

There is no need to #define the license of the driver, just put it in
the MODULE_LICENSE() line directly as a text string.
This allows tools that check that the module license matches the source
code license to work properly, as there is no need to unwind the
unneeded dereference.

[]

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c

[]

@@ -76,7 +76,6 @@
#define MOD_AUTHOR "Option Wireless"
#define MOD_DESCRIPTION "USB High Speed Option driver"
-#define MOD_LICENSE "GPL"
#define HSO_MAX_NET_DEVICES 10
#define HSO__MAX_MTU 2048
@@ -3288,7 +3287,7 @@ module_exit(hso_exit);
MODULE_AUTHOR(MOD_AUTHOR);
MODULE_DESCRIPTION(MOD_DESCRIPTION);
-MODULE_LICENSE(MOD_LICENSE);
+MODULE_LICENSE("GPL");

Probably all of these MODULE_<FOO>(MOD_<BAR>) uses could be
simplified as well.

Agreed, I did that for a bunch of USB drivers, need to do it for others
as well.

thanks,

greg k-h

Julia Lawall

2017-11-22 17:21:34 UTC

Permalink

[]

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c

[]

Probably all of these MODULE_<FOO>(MOD_<BAR>) uses could be
simplified as well.
Perhaps there's utility in a (cocci?) script that looks for
used-once
macro #defines in various types of macros.

It could be possible. It's a bit tricky due to ifdefs that Coccinelle
doesn't see and header files, but perhaps in special cases like this there
is not much worry.

julia

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1511370336.6989.100.camel%40perches.com.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2017-11-22 21:20:49 UTC

Permalink

[]

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c

[]

Probably all of these MODULE_<FOO>(MOD_<BAR>) uses could be
simplified as well.
Perhaps there's utility in a (cocci?) script that looks for
used-once
macro #defines in various types of macros.

What about module_version, eg:

diff -u -p a/drivers/ata/pata_pdc202xx_old.c
b/drivers/ata/pata_pdc202xx_old.c
--- a/drivers/ata/pata_pdc202xx_old.c
+++ b/drivers/ata/pata_pdc202xx_old.c
@@ -21,7 +21,6 @@
#include <linux/libata.h>

#define DRV_NAME "pata_pdc202xx_old"
-#define DRV_VERSION "0.4.3"

static int pdc2026x_cable_detect(struct ata_port *ap)
{
@@ -389,4 +388,4 @@ MODULE_AUTHOR("Alan Cox");
MODULE_DESCRIPTION("low-level driver for Promise 2024x and 20262-20267");
MODULE_LICENSE("GPL");
MODULE_DEVICE_TABLE(pci, pdc202xx);
-MODULE_VERSION(DRV_VERSION);
+MODULE_VERSION("0.4.3");

julia

Greg Kroah-Hartman

2017-11-23 06:22:28 UTC

Permalink

Post by Julia Lawall

[]

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c

[]

Probably all of these MODULE_<FOO>(MOD_<BAR>) uses could be
simplified as well.
Perhaps there's utility in a (cocci?) script that looks for
used-once
macro #defines in various types of macros.

diff -u -p a/drivers/ata/pata_pdc202xx_old.c
b/drivers/ata/pata_pdc202xx_old.c
--- a/drivers/ata/pata_pdc202xx_old.c
+++ b/drivers/ata/pata_pdc202xx_old.c
@@ -21,7 +21,6 @@
#include <linux/libata.h>
#define DRV_NAME "pata_pdc202xx_old"
-#define DRV_VERSION "0.4.3"
static int pdc2026x_cable_detect(struct ata_port *ap)
{
@@ -389,4 +388,4 @@ MODULE_AUTHOR("Alan Cox");
MODULE_DESCRIPTION("low-level driver for Promise 2024x and 20262-20267");
MODULE_LICENSE("GPL");
MODULE_DEVICE_TABLE(pci, pdc202xx);
-MODULE_VERSION(DRV_VERSION);
+MODULE_VERSION("0.4.3");

I've just deleted MODULE_VERSION() entirely from some subsystems, as
once the driver is in the kernel source tree, the "version" makes almost
no sense at all.

But I know some companies love incrementing it (some network and scsi
drivers specifically), so those might want to keep it around for some
odd reason.

thanks

greg k-h

Julia Lawall

2017-11-23 07:31:43 UTC

Permalink

Post by Greg Kroah-Hartman

Post by Julia Lawall

[]

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c

[]

Probably all of these MODULE_<FOO>(MOD_<BAR>) uses could be
simplified as well.
Perhaps there's utility in a (cocci?) script that looks for
used-once
macro #defines in various types of macros.

I've just deleted MODULE_VERSION() entirely from some subsystems, as
once the driver is in the kernel source tree, the "version" makes almost
no sense at all.
But I know some companies love incrementing it (some network and scsi
drivers specifically), so those might want to keep it around for some
odd reason.

OK, that seems like a simple soluton. Thanks.

julia

Joe Perches

2017-11-23 23:30:01 UTC

Permalink

Post by Greg Kroah-Hartman

[]

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c

[]

Probably all of these MODULE_<FOO>(MOD_<BAR>) uses could be
simplified as well.

Agreed, I did that for a bunch of USB drivers, need to do it for others
as well.

Here's a little perl and bash script that seems to do the right thing
---
--- /dev/null 2017-11-23 06:19:12.943046739 -0800
+++ single_use_module.pl 2017-11-23 15:23:11.729812156 -0800
@@ -0,0 +1,15 @@
+$/ = undef;
+my $var = $ARGV[0];
+my $file = $ARGV[1];
+print("var: <$var> file: <$file>\n");
+open my $fh, "<", $file or die;
+my $data = <$fh>;
+close $fh;
+$data =~ s/\n#[ \t]*define\s+$var\s+(.*)\n/\n/;
+my $string = $1;
+print("string: <$string>\n");
+$string =~ s/\s+\n//;
+$data =~ s~$var~$string~;
+open my $fh, ">", $file or die;
+print $fh $data;
+close $fh;

--- /dev/null 2017-11-23 06:19:12.943046739 -0800
+++ single_use_module.bash 2017-11-23 15:23:01.964676948 -0800
@@ -0,0 +1,25 @@
+#!/bin/bash
+git grep -P '\bMODULE_[A-Z]+\s*$\s*[A-Z_]+\s*$' $@ |
+ while read line ; do
+ file=$(echo $line | cut -f1 -d":")
+ define=$(echo $line | cut -f2- -d":")
+ var=$(echo $define | sed -r -e 's/^MODULE_[A-Z_]+\s*\(\s*//' -r -e 's/^([A-Z_]+).*$/\1/')
+
+ # see if the define exists in the file
+ count1=$(git grep -c -P "^\s*#\s*define\s+$var\s+" $file | cut -f2- -d":")
+ if [[ $count1 != 1 ]] ; then
+ continue
+ fi
+
+ # see if the var exists twice (once in the #define, once in the use)
+ count2=$(git grep -c -P -w $var $file | cut -f2- -d":")
+ if [[ $count2 != 2 ]] ; then
+ continue
+ fi
+
+ if [[ "${defline: -1}" == "\\" ]] ; then
+ continue
+ fi
+
+ perl single_use_module.pl $var $file
+ done

Joe Perches

2017-11-24 00:25:37 UTC

Permalink

Post by Joe Perches
--- /dev/null 2017-11-23 06:19:12.943046739 -0800
+++ single_use_module.pl 2017-11-23 15:23:11.729812156 -0800
@@ -0,0 +1,15 @@

[]

Post by Joe Perches
+$data =~ s~$var~$string~;

this needs to be:

$data =~ s~\b$var\b~$string~;