Skip to contentSkip to conversation

ipmi: fix module param sparse warnings

PATCH In review Randy Dunlap 2 participants 4 messages Jun 24, 2010 – Jun 28, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ipmi: fix module param sparse warnings

From: Randy Dunlap <[email protected]>

The <arg> parameter in module_param_cb() should be a pointer, i.e., like one of these:

	union {
		void *arg;
		const struct kparam_string *str;
		const struct kparam_array *arr;
	};

sparse complains:
drivers/char/ipmi/ipmi_watchdog.c:303:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:307:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:311:1: error: cannot dereference this type

---
Note: patch is against linux-next, since these lines have changed recently.

 drivers/char/ipmi/ipmi_watchdog.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/char/ipmi/ipmi_watchdog.c +3 -3
@@ -300,15 +300,15 @@ MODULE_PARM_DESC(timeout, "Timeout value
 module_param(pretimeout, timeout, 0644);
 MODULE_PARM_DESC(pretimeout, "Pretimeout value in seconds.");
 
-module_param_cb(action, &param_ops_str, action_op, 0644);
+module_param_cb(action, &param_ops_str, &action_op, 0644);
 MODULE_PARM_DESC(action, "Timeout action. One of: "
 		 "reset, none, power_cycle, power_off.");
 
-module_param_cb(preaction, &param_ops_str, preaction_op, 0644);
+module_param_cb(preaction, &param_ops_str, &preaction_op, 0644);
 MODULE_PARM_DESC(preaction, "Pretimeout action.  One of: "
 		 "pre_none, pre_smi, pre_nmi, pre_int.");
 
-module_param_cb(preop, &param_ops_str, preop_op, 0644);
+module_param_cb(preop, &param_ops_str, &preop_op, 0644);
 MODULE_PARM_DESC(preop, "Pretimeout driver operation.  One of: "
 		 "preop_none, preop_panic, preop_give_data.");
 
#1 lore raw
Quoted 3 lines · Randy Dunlap

From: Randy Dunlap <[email protected]>

The <arg> parameter in module_param_cb() should be a pointer,

Well, I don't think there's anything wrong with it being a function.

The actual problem is that it tries to figure out if the arg is a bool or an int: if someone wants to work through the 700-odd cases of module_param*bool and convert those vars to actual bools, that'd allow us to get rid of that logic.

Meanwhile, we can do this instead. Does it solve it as well?

Subject: ipmi: fix module param sparse warnings
Date: Thu, 24 Jun 2010 15:21:22 -0700
From: Randy Dunlap <[email protected]>

The <arg> parameter in module_param_cb() should be a pointer, i.e., like one of these:

	union {
		void *arg;
		const struct kparam_string *str;
		const struct kparam_array *arr;
	};

sparse complains:
drivers/char/ipmi/ipmi_watchdog.c:303:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:307:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:311:1: error: cannot dereference this type

---
 drivers/char/ipmi/ipmi_watchdog.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/char/ipmi/ipmi_watchdog.c +3 -3
@@ -300,15 +300,15 @@ MODULE_PARM_DESC(timeout, "Timeout value
 module_param(pretimeout, timeout, 0644);
 MODULE_PARM_DESC(pretimeout, "Pretimeout value in seconds.");
 
-module_param_cb(action, &param_ops_str, action_op, 0644);
+module_param_cb(action, &param_ops_str, &action_op, 0644);
 MODULE_PARM_DESC(action, "Timeout action. One of: "
 		 "reset, none, power_cycle, power_off.");
 
-module_param_cb(preaction, &param_ops_str, preaction_op, 0644);
+module_param_cb(preaction, &param_ops_str, &preaction_op, 0644);
 MODULE_PARM_DESC(preaction, "Pretimeout action.  One of: "
 		 "pre_none, pre_smi, pre_nmi, pre_int.");
 
-module_param_cb(preop, &param_ops_str, preop_op, 0644);
+module_param_cb(preop, &param_ops_str, &preop_op, 0644);
 MODULE_PARM_DESC(preop, "Pretimeout driver operation.  One of: "
 		 "preop_none, preop_panic, preop_give_data.");
 
#2 lore raw
Quoted 6 lines · Rusty Russell
Quoted 3 lines · Randy Dunlap

From: Randy Dunlap <[email protected]>

The <arg> parameter in module_param_cb() should be a pointer,

Well, I don't think there's anything wrong with it being a function.

Ah, yes, I see. That makes it a sparse error, right?

Quoted 6 lines

The actual problem is that it tries to figure out if the arg is a bool or an int: if someone wants to work through the 700-odd cases of module_param*bool and convert those vars to actual bools, that'd allow us to get rid of that logic.

Meanwhile, we can do this instead. Does it solve it as well?

Yes, that also fixes it.
Thanks.

Quoted 47 lines

Subject: ipmi: fix module param sparse warnings
Date: Thu, 24 Jun 2010 15:21:22 -0700
From: Randy Dunlap <[email protected]>

The <arg> parameter in module_param_cb() should be a pointer, i.e., like one of these:

	union {
		void *arg;
		const struct kparam_string *str;
		const struct kparam_array *arr;
	};

sparse complains:
drivers/char/ipmi/ipmi_watchdog.c:303:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:307:1: error: cannot dereference this type drivers/char/ipmi/ipmi_watchdog.c:311:1: error: cannot dereference this type

---
 drivers/char/ipmi/ipmi_watchdog.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/char/ipmi/ipmi_watchdog.c +3 -3
@@ -300,15 +300,15 @@ MODULE_PARM_DESC(timeout, "Timeout value
 module_param(pretimeout, timeout, 0644);
 MODULE_PARM_DESC(pretimeout, "Pretimeout value in seconds.");
 
-module_param_cb(action, &param_ops_str, action_op, 0644);
+module_param_cb(action, &param_ops_str, &action_op, 0644);
 MODULE_PARM_DESC(action, "Timeout action. One of: "
 		 "reset, none, power_cycle, power_off.");
 
-module_param_cb(preaction, &param_ops_str, preaction_op, 0644);
+module_param_cb(preaction, &param_ops_str, &preaction_op, 0644);
 MODULE_PARM_DESC(preaction, "Pretimeout action.  One of: "
 		 "pre_none, pre_smi, pre_nmi, pre_int.");
 
-module_param_cb(preop, &param_ops_str, preop_op, 0644);
+module_param_cb(preop, &param_ops_str, &preop_op, 0644);
 MODULE_PARM_DESC(preop, "Pretimeout driver operation.  One of: "
 		 "preop_none, preop_panic, preop_give_data.");
 
Signature · 2 lines
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
#3 lore raw
Quoted 9 lines · Randy Dunlap
Quoted 6 lines · Rusty Russell
Quoted 3 lines · Randy Dunlap

From: Randy Dunlap <[email protected]>

The <arg> parameter in module_param_cb() should be a pointer,

Well, I don't think there's anything wrong with it being a function.

Ah, yes, I see. That makes it a sparse error, right?

Well, it's a weird corner case, but yes.

Quoted 4 lines
Quoted 1 line

Meanwhile, we can do this instead. Does it solve it as well?

Yes, that also fixes it.
Thanks.

Thanks, I've added your tested-by.

Cheers,
Rusty.