[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-thinkpad
Subject:    Re: [ltp] [PATCH 1/1] force_io is of type int, so check for this type instead of bool.
From:       Bjørn_Mork <bjorn () mork ! no>
Date:       2013-04-18 7:56:26
Message-ID: 871ua8tgph.fsf () nemi ! mork ! no
[Download RAW message or body]

Joerg Mayer <jmayer@loplof.de> writes:

> Fixes the following warning:
> 
> CC [M]  /home/jmayer/work/thinkpad/git/thinkpad_ec.o
> In file included from /usr/src/linux-3.8.7-1/include/linux/module.h:17:0,
> from /home/jmayer/work/thinkpad/git/thinkpad_ec.c:33:
> /home/jmayer/work/thinkpad/git/thinkpad_ec.c: In function ‘__check_force_io':
> /usr/src/linux-3.8.7-1/include/linux/moduleparam.h:338:45: warning: return from \
> incompatible pointer type [enabled by default] static inline type \
> *__check_##name(void) { return(p); } ^
> /usr/src/linux-3.8.7-1/include/linux/moduleparam.h:384:35: note: in expansion of \
> macro ‘__param_check' #define param_check_bool(name, p) __param_check(name, p, \
> bool) ^
> /usr/src/linux-3.8.7-1/include/linux/moduleparam.h:116:2: note: in expansion of \
> macro ‘param_check_bool' param_check_##type(name, &(value));       \
> ^
> /home/jmayer/work/thinkpad/git/thinkpad_ec.c:100:1: note: in expansion of macro \
> ‘module_param_named' module_param_named(force_io, force_io, bool, 0600);
> ^
> 
> Signed-off-by: Joerg Mayer <jmayer@loplof.de>
> ---
> > 100644 100644 a1c94d9... ea8bcbf... M	thinkpad_ec.c
> thinkpad_ec.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/thinkpad_ec.c b/thinkpad_ec.c
> index a1c94d9..ea8bcbf 100644
> --- a/thinkpad_ec.c
> +++ b/thinkpad_ec.c
> @@ -97,7 +97,7 @@ static DEFINE_SEMAPHORE(thinkpad_ec_mutex);
> /* Kludge in case the ACPI DSDT reserves the ports we need. */
> static int force_io;    /* Willing to do IO to ports we couldn't reserve? */
> static int reserved_io; /* Successfully reserved the ports? */
> -module_param_named(force_io, force_io, bool, 0600);
> +module_param_named(force_io, force_io, int, 0600);
> MODULE_PARM_DESC(force_io, "Force IO even if region already reserved (0=off, \
> 1=on)");

Wouldn't it be better to change the type of force_io to bool?

We usually prefer that to integers for any boolean variable, and keeping
the module_param bool has the advantage that you can use the 'Y' or 'N'
aliases instead of '0' or '1'.

BTW, using the "_named" variant of the macro seems unnecessary here,
given that the parameter and variable have the same name...


Bjørn
--
The linux-thinkpad mailing list home page is at:
http://mailman.linux-thinkpad.org/mailman/listinfo/linux-thinkpad


[prev in list] [next in list] [prev in thread] [next in thread]