Re: Patch "paride: make 'verbose' parameter an 'int' again" has been added to the 3.14-stable tree

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 03 May 2016 07:04:10 Rusty Russell wrote:
> gregkh@linuxfoundation.org writes:
> > This is a note to let you know that I've just added the patch titled
> >
> >     paride: make 'verbose' parameter an 'int' again
> >
> > to the 3.14-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> >      paride-make-verbose-parameter-an-int-again.patch
> > and it can be found in the queue-3.14 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >
> >
> >>From dec63a4dec2d6d01346fd5d96062e67c0636852b Mon Sep 17 00:00:00 2001
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Tue, 15 Mar 2016 14:53:29 -0700
> > Subject: paride: make 'verbose' parameter an 'int' again
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > commit dec63a4dec2d6d01346fd5d96062e67c0636852b upstream.
> >
> > gcc-6.0 found an ancient bug in the paride driver, which had a
> > "module_param(verbose, bool, 0);" since before 2.6.12, but actually uses
> > it to accept '0', '1' or '2' as arguments:
> >
> >   drivers/block/paride/pd.c: In function 'pd_init_dev_parms':
> >   drivers/block/paride/pd.c:298:29: warning: comparison of constant '1' with boolean expression is always false [-Wbool-compare]
> >    #define DBMSG(msg) ((verbose>1)?(msg):NULL)
> >
> > In 2012, Rusty did a cleanup patch that also changed the type of the
> > variable to 'bool', which introduced what is now a gcc warning.
> >
> > This changes the type back to 'int' and adapts the module_param() line
> > instead, so it should work as documented in case anyone ever cares about
> > running the ancient driver with debugging.
> >
> > Fixes: 90ab5ee94171 ("module_param: make bool parameters really bool (drivers & misc)")
> 
> Hmm, I got CC'd on this, not the original 

Sorry about that, I try to Cc people manually when I have a fixes tag, but
I sometimes forget. I should update my script to do that automatically.

> Because it *never* worked.  90ab5ee94171 changed it to a bool, but it
> was already module_param(bool) so you could never set it to 2.  ... And
> it's been module_param(bool) before 2005.
> 
> But this breaks anyone who has been using just "verbose" or "verbose=y".
> 
> Fortunately, I'm sure nobody cares about it?

Right. I think we can safely make that assumption that nobody cares
about the new change in behavior. As I tried to explain in the changelog,
the bug of not being able to set verbose=2 is historic, and I am
not worried about it. Your patch from 2012 makes the driver more
consistent, but it does introduce what has now become a gcc warning.

We probably don't care about that in backports, because nobody uses
the driver and people rarely build older kernels with the latest
gcc release, if that's even possible.

FWIW, I've managed to find how the odd behavior was introduced originally
in linux-2.6.11 just before the start of git, see below for giggles.
I'm also sure that nobody cared about the verbose=2 syntax breaking back
then, it was already obsolete technology even at the turn of the century.

	Arnd

---
commit 3352539ac7e770a69b003f4a0952f817695b9204
Author: Andrew Morton <akpm@osdl.org>
Date:   Fri Jan 7 22:21:50 2005 -0800

    [PATCH] MODULE_PARM conversions
    
    Rusty Russell <rusty@rustcorp.com.au>
    
    MODULE_PARM conversions for x86 `allyesconfig'.
    
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index 5f6534d..c4c8f0d 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -178,15 +178,15 @@ void pd_setup(char *str, int *ints)
 
 #endif
 
-MODULE_PARM(verbose, "i");
-MODULE_PARM(major, "i");
-MODULE_PARM(name, "s");
-MODULE_PARM(cluster, "i");
-MODULE_PARM(nice, "i");
-MODULE_PARM(drive0, "1-8i");
-MODULE_PARM(drive1, "1-8i");
-MODULE_PARM(drive2, "1-8i");
-MODULE_PARM(drive3, "1-8i");
+module_param(verbose, bool, 0);
+module_param(major, int, 0);
+module_param(name, charp, 0);
+module_param(cluster, int, 0);
+module_param(nice, int, 0);
+module_param_array(drive0, int, NULL, 0);
+module_param_array(drive1, int, NULL, 0);
+module_param_array(drive2, int, NULL, 0);
+module_param_array(drive3, int, NULL, 0);
 
 #include "paride.h"