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

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

[Greg Kroah-Hartman]

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.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andreas Kemnade <andreas@kemnade.info>
Cc: Johan Hovold <johan@kernel.org>
Reported-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/usb/hso.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index d7a3379ea668..f40d1423cf46 100644
--- 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");
 
 /* change the debug level (eg: insmod hso.ko debug=0x04) */
 MODULE_PARM_DESC(debug, "debug level mask [0x01 | 0x02 | 0x04 | 0x08 | 0x10]");
-- 
2.15.0

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

[Philippe Ombredanne]

On Fri, Nov 17, 2017 at 3:19 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> 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.
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Andreas Kemnade <andreas@kemnade.info>
> Cc: Johan Hovold <johan@kernel.org>
> Reported-by: Philippe Ombredanne <pombredanne@nexb.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
-- 
Cordially
Philippe Ombredanne

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

[David Miller]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Fri, 17 Nov 2017 15:19:39 +0100

> 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.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Andreas Kemnade <andreas@kemnade.info>
> Cc: Johan Hovold <johan@kernel.org>
> Reported-by: Philippe Ombredanne <pombredanne@nexb.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Applied.

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

[Joe Perches]

On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> 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.

Perhaps there's utility in a (cocci?) script that looks for
used-once
macro #defines in various types of macros.

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

[Greg Kroah-Hartman]

On Wed, Nov 22, 2017 at 09:05:36AM -0800, Joe Perches wrote:
> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > 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

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

[Joe Perches]

On Wed, 2017-11-22 at 18:12 +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 22, 2017 at 09:05:36AM -0800, Joe Perches wrote:
> > On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > > 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.

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

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

[Joe Perches]

On Thu, 2017-11-23 at 15:30 -0800, Joe Perches wrote:
> --- /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 @@
[]
> +$data =~ s~$var~$string~;

this needs to be:

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

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Wed, 22 Nov 2017, Joe Perches wrote:

> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > 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.
>
> 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 unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> 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.
>

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Wed, 22 Nov 2017, Joe Perches wrote:

> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > 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.
>
> 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

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Greg Kroah-Hartman]

On Wed, Nov 22, 2017 at 10:20:49PM +0100, Julia Lawall wrote:
> 
> 
> On Wed, 22 Nov 2017, Joe Perches wrote:
> 
> > On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > > 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.
> >
> > 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");

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

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Thu, 23 Nov 2017, Greg Kroah-Hartman wrote:

> On Wed, Nov 22, 2017 at 10:20:49PM +0100, Julia Lawall wrote:
> >
> >
> > On Wed, 22 Nov 2017, Joe Perches wrote:
> >
> > > On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > > > 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.
> > >
> > > 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");
>
> 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