* [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...)
@ 2012-05-03 19:12 Jim Cromie
2012-05-03 19:23 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Jim Cromie @ 2012-05-03 19:12 UTC (permalink / raw)
To: gregkh, linux-kernel; +Cc: jbaron, Jim Cromie, Joe Perches
I left 1 printk which uses __FILE__, __LINE__ explicitly, which should
not be subject to generic preferences expressed via pr_fmt().
+ tweaks suggested by Joe Perches:
- add doing to irq-enabled warning, like others. It wont happen often..
- change sysfs failure crit, not just err, make it 1 line in logs.
cc: Joe Perches <joe@perches.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
kernel/params.c | 34 +++++++++++++---------------------
1 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index be78c90..2332e0b 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -201,25 +201,22 @@ int parse_args(const char *doing,
irq_was_disabled = irqs_disabled();
ret = parse_one(param, val, doing, params, num,
min_level, max_level, unknown);
- if (irq_was_disabled && !irqs_disabled()) {
- printk(KERN_WARNING "parse_args(): option '%s' enabled "
- "irq's!\n", param);
- }
+ if (irq_was_disabled && !irqs_disabled())
+ pr_warn("%s: option '%s' enabled irq's!\n",
+ doing, param);
+
switch (ret) {
case -ENOENT:
- printk(KERN_ERR "%s: Unknown parameter `%s'\n",
- doing, param);
+ pr_err("%s: Unknown parameter `%s'\n", doing, param);
return ret;
case -ENOSPC:
- printk(KERN_ERR
- "%s: `%s' too large for parameter `%s'\n",
+ pr_err("%s: `%s' too large for parameter `%s'\n",
doing, val ?: "", param);
return ret;
case 0:
break;
default:
- printk(KERN_ERR
- "%s: `%s' invalid for parameter `%s'\n",
+ pr_err("%s: `%s' invalid for parameter `%s'\n",
doing, val ?: "", param);
return ret;
}
@@ -266,8 +263,7 @@ STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);
int param_set_charp(const char *val, const struct kernel_param *kp)
{
if (strlen(val) > 1024) {
- printk(KERN_ERR "%s: string parameter too long\n",
- kp->name);
+ pr_err("%s: string parameter too long\n", kp->name);
return -ENOSPC;
}
@@ -403,8 +399,7 @@ static int param_array(const char *name,
int len;
if (*num == max) {
- printk(KERN_ERR "%s: can only take %i arguments\n",
- name, max);
+ pr_err("%s: can only take %i arguments\n", name, max);
return -EINVAL;
}
len = strcspn(val, ",");
@@ -423,8 +418,7 @@ static int param_array(const char *name,
} while (save == ',');
if (*num < min) {
- printk(KERN_ERR "%s: needs at least %i arguments\n",
- name, min);
+ pr_err("%s: needs at least %i arguments\n", name, min);
return -EINVAL;
}
return 0;
@@ -483,7 +477,7 @@ int param_set_copystring(const char *val, const struct kernel_param *kp)
const struct kparam_string *kps = kp->str;
if (strlen(val)+1 > kps->maxlen) {
- printk(KERN_ERR "%s: string doesn't fit in %u chars.\n",
+ pr_err("%s: string doesn't fit in %u chars.\n",
kp->name, kps->maxlen-1);
return -ENOSPC;
}
@@ -753,11 +747,9 @@ static struct module_kobject * __init locate_module_kobject(const char *name)
#endif
if (err) {
kobject_put(&mk->kobj);
- printk(KERN_ERR
- "Module '%s' failed add to sysfs, error number %d\n",
+ pr_crit("Adding module '%s' to sysfs failed (%d), "
+ "the system may be unstable.\n",
name, err);
- printk(KERN_ERR
- "The system will be unstable now.\n");
return NULL;
}
--
1.7.8.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...)
2012-05-03 19:12 [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...) Jim Cromie
@ 2012-05-03 19:23 ` Joe Perches
0 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2012-05-03 19:23 UTC (permalink / raw)
To: Jim Cromie; +Cc: gregkh, linux-kernel, jbaron
On Thu, 2012-05-03 at 13:12 -0600, Jim Cromie wrote:
> I left 1 printk which uses __FILE__, __LINE__ explicitly, which should
> not be subject to generic preferences expressed via pr_fmt().
[]
> diff --git a/kernel/params.c b/kernel/params.c
[]
> @@ -753,11 +747,9 @@ static struct module_kobject * __init locate_module_kobject(const char *name)
> #endif
> if (err) {
> kobject_put(&mk->kobj);
> - printk(KERN_ERR
> - "Module '%s' failed add to sysfs, error number %d\n",
> + pr_crit("Adding module '%s' to sysfs failed (%d), "
> + "the system may be unstable.\n",
> name, err);
> - printk(KERN_ERR
> - "The system will be unstable now.\n");
> return NULL;
> }
>
It'd be nicer to coalesce the formats and not worry about 80 columns.
Coalesced formats are easier to find via grep.
pr_crit("Adding module '%s' to sysfs failed (%d), the system may be unstable\n",
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...)
@ 2012-05-04 0:22 Jim Cromie
0 siblings, 0 replies; 5+ messages in thread
From: Jim Cromie @ 2012-05-04 0:22 UTC (permalink / raw)
To: gregkh, linux-kernel; +Cc: jbaron, Jim Cromie, Joe Perches
I left 1 printk which uses __FILE__, __LINE__ explicitly, which should
not be subject to generic preferences expressed via pr_fmt().
+ tweaks suggested by Joe Perches:
- add doing to irq-enabled warning, like others. It wont happen often..
- change sysfs failure crit, not just err, make it 1 line in logs.
- coalese 2 format fragments into 1 >80 char line
cc: Joe Perches <joe@perches.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
kernel/params.c | 33 ++++++++++++---------------------
1 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index be78c90..ed35345 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -201,25 +201,22 @@ int parse_args(const char *doing,
irq_was_disabled = irqs_disabled();
ret = parse_one(param, val, doing, params, num,
min_level, max_level, unknown);
- if (irq_was_disabled && !irqs_disabled()) {
- printk(KERN_WARNING "parse_args(): option '%s' enabled "
- "irq's!\n", param);
- }
+ if (irq_was_disabled && !irqs_disabled())
+ pr_warn("%s: option '%s' enabled irq's!\n",
+ doing, param);
+
switch (ret) {
case -ENOENT:
- printk(KERN_ERR "%s: Unknown parameter `%s'\n",
- doing, param);
+ pr_err("%s: Unknown parameter `%s'\n", doing, param);
return ret;
case -ENOSPC:
- printk(KERN_ERR
- "%s: `%s' too large for parameter `%s'\n",
+ pr_err("%s: `%s' too large for parameter `%s'\n",
doing, val ?: "", param);
return ret;
case 0:
break;
default:
- printk(KERN_ERR
- "%s: `%s' invalid for parameter `%s'\n",
+ pr_err("%s: `%s' invalid for parameter `%s'\n",
doing, val ?: "", param);
return ret;
}
@@ -266,8 +263,7 @@ STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);
int param_set_charp(const char *val, const struct kernel_param *kp)
{
if (strlen(val) > 1024) {
- printk(KERN_ERR "%s: string parameter too long\n",
- kp->name);
+ pr_err("%s: string parameter too long\n", kp->name);
return -ENOSPC;
}
@@ -403,8 +399,7 @@ static int param_array(const char *name,
int len;
if (*num == max) {
- printk(KERN_ERR "%s: can only take %i arguments\n",
- name, max);
+ pr_err("%s: can only take %i arguments\n", name, max);
return -EINVAL;
}
len = strcspn(val, ",");
@@ -423,8 +418,7 @@ static int param_array(const char *name,
} while (save == ',');
if (*num < min) {
- printk(KERN_ERR "%s: needs at least %i arguments\n",
- name, min);
+ pr_err("%s: needs at least %i arguments\n", name, min);
return -EINVAL;
}
return 0;
@@ -483,7 +477,7 @@ int param_set_copystring(const char *val, const struct kernel_param *kp)
const struct kparam_string *kps = kp->str;
if (strlen(val)+1 > kps->maxlen) {
- printk(KERN_ERR "%s: string doesn't fit in %u chars.\n",
+ pr_err("%s: string doesn't fit in %u chars.\n",
kp->name, kps->maxlen-1);
return -ENOSPC;
}
@@ -753,11 +747,8 @@ static struct module_kobject * __init locate_module_kobject(const char *name)
#endif
if (err) {
kobject_put(&mk->kobj);
- printk(KERN_ERR
- "Module '%s' failed add to sysfs, error number %d\n",
+ pr_crit("Adding module '%s' to sysfs failed (%d), the system may be unstable.\n",
name, err);
- printk(KERN_ERR
- "The system will be unstable now.\n");
return NULL;
}
--
1.7.8.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 1/3] params.c: fix Smack complaint about parse_args
@ 2012-05-03 17:57 Jim Cromie
2012-05-03 17:57 ` [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...) Jim Cromie
0 siblings, 1 reply; 5+ messages in thread
From: Jim Cromie @ 2012-05-03 17:57 UTC (permalink / raw)
To: gregkh, linux-kernel; +Cc: jbaron, Jim Cromie, Dan Carpenter
In commit 9fb48c744: "params: add 3rd arg to option handler callback
signature", the if-guard added to the pr_debug was overzealous; no
callers pass NULL, and existing code above and below the guard assumes
as much. Change the if-guard to match, and silence the Smack
complaint.
CC: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
kernel/params.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index b60e2c7..be78c90 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -190,7 +190,7 @@ int parse_args(const char *doing,
/* Chew leading spaces */
args = skip_spaces(args);
- if (args && *args)
+ if (*args)
pr_debug("doing %s, parsing ARGS: '%s'\n", doing, args);
while (*args) {
--
1.7.8.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...)
2012-05-03 17:57 [PATCH 1/3] params.c: fix Smack complaint about parse_args Jim Cromie
@ 2012-05-03 17:57 ` Jim Cromie
2012-05-03 18:50 ` Joe Perches
0 siblings, 1 reply; 5+ messages in thread
From: Jim Cromie @ 2012-05-03 17:57 UTC (permalink / raw)
To: gregkh, linux-kernel; +Cc: jbaron, Jim Cromie
I left 1 printk which uses __FILE__, __LINE__ explicitly, which should
not be subject to generic preferences expressed via pr_fmt().
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
kernel/params.c | 33 ++++++++++++---------------------
1 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index be78c90..08414ba 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -201,25 +201,21 @@ int parse_args(const char *doing,
irq_was_disabled = irqs_disabled();
ret = parse_one(param, val, doing, params, num,
min_level, max_level, unknown);
- if (irq_was_disabled && !irqs_disabled()) {
- printk(KERN_WARNING "parse_args(): option '%s' enabled "
- "irq's!\n", param);
- }
+ if (irq_was_disabled && !irqs_disabled())
+ pr_warn("option '%s' enabled irq's!\n", param);
+
switch (ret) {
case -ENOENT:
- printk(KERN_ERR "%s: Unknown parameter `%s'\n",
- doing, param);
+ pr_err("%s: Unknown parameter `%s'\n", doing, param);
return ret;
case -ENOSPC:
- printk(KERN_ERR
- "%s: `%s' too large for parameter `%s'\n",
+ pr_err("%s: `%s' too large for parameter `%s'\n",
doing, val ?: "", param);
return ret;
case 0:
break;
default:
- printk(KERN_ERR
- "%s: `%s' invalid for parameter `%s'\n",
+ pr_err("%s: `%s' invalid for parameter `%s'\n",
doing, val ?: "", param);
return ret;
}
@@ -266,8 +262,7 @@ STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);
int param_set_charp(const char *val, const struct kernel_param *kp)
{
if (strlen(val) > 1024) {
- printk(KERN_ERR "%s: string parameter too long\n",
- kp->name);
+ pr_err("%s: string parameter too long\n", kp->name);
return -ENOSPC;
}
@@ -403,8 +398,7 @@ static int param_array(const char *name,
int len;
if (*num == max) {
- printk(KERN_ERR "%s: can only take %i arguments\n",
- name, max);
+ pr_err("%s: can only take %i arguments\n", name, max);
return -EINVAL;
}
len = strcspn(val, ",");
@@ -423,8 +417,7 @@ static int param_array(const char *name,
} while (save == ',');
if (*num < min) {
- printk(KERN_ERR "%s: needs at least %i arguments\n",
- name, min);
+ pr_err("%s: needs at least %i arguments\n", name, min);
return -EINVAL;
}
return 0;
@@ -483,7 +476,7 @@ int param_set_copystring(const char *val, const struct kernel_param *kp)
const struct kparam_string *kps = kp->str;
if (strlen(val)+1 > kps->maxlen) {
- printk(KERN_ERR "%s: string doesn't fit in %u chars.\n",
+ pr_err("%s: string doesn't fit in %u chars.\n",
kp->name, kps->maxlen-1);
return -ENOSPC;
}
@@ -753,11 +746,9 @@ static struct module_kobject * __init locate_module_kobject(const char *name)
#endif
if (err) {
kobject_put(&mk->kobj);
- printk(KERN_ERR
- "Module '%s' failed add to sysfs, error number %d\n",
+ pr_err("Module '%s' failed add to sysfs, error: %d\n"
+ "The system will be unstable now.\n",
name, err);
- printk(KERN_ERR
- "The system will be unstable now.\n");
return NULL;
}
--
1.7.8.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...)
2012-05-03 17:57 ` [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...) Jim Cromie
@ 2012-05-03 18:50 ` Joe Perches
0 siblings, 0 replies; 5+ messages in thread
From: Joe Perches @ 2012-05-03 18:50 UTC (permalink / raw)
To: Jim Cromie; +Cc: gregkh, linux-kernel, jbaron
On Thu, 2012-05-03 at 11:57 -0600, Jim Cromie wrote:
> I left 1 printk which uses __FILE__, __LINE__ explicitly, which should
> not be subject to generic preferences expressed via pr_fmt().
>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
> kernel/params.c | 33 ++++++++++++---------------------
> 1 files changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index be78c90..08414ba 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -201,25 +201,21 @@ int parse_args(const char *doing,
> irq_was_disabled = irqs_disabled();
> ret = parse_one(param, val, doing, params, num,
> min_level, max_level, unknown);
> - if (irq_was_disabled && !irqs_disabled()) {
> - printk(KERN_WARNING "parse_args(): option '%s' enabled "
> - "irq's!\n", param);
> - }
> + if (irq_was_disabled && !irqs_disabled())
> + pr_warn("option '%s' enabled irq's!\n", param);
> +
The other parse_args pr_<level> uses have
'"%s: ...", doing, ...'.
Maybe this one should too.
> switch (ret) {
> case -ENOENT:
> - printk(KERN_ERR "%s: Unknown parameter `%s'\n",
> - doing, param);
> + pr_err("%s: Unknown parameter `%s'\n", doing, param);
> return ret;
[]
> @@ -753,11 +746,9 @@ static struct module_kobject * __init locate_module_kobject(const char *name)
> #endif
> if (err) {
> kobject_put(&mk->kobj);
> - printk(KERN_ERR
> - "Module '%s' failed add to sysfs, error number %d\n",
> + pr_err("Module '%s' failed add to sysfs, error: %d\n"
> + "The system will be unstable now.\n",
> name, err);
> - printk(KERN_ERR
> - "The system will be unstable now.\n");
> return NULL;
> }
>
It'd be nice to align the arguments and perhaps a
single line output may be better.
Maybe something like:
pr_err("Adding module '%s' to sysfs failed (%d), the system is unstable\n",
name, err);
though perhaps this should be pr_crit
if it's really unstable.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-05-04 0:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-03 19:12 [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...) Jim Cromie
2012-05-03 19:23 ` Joe Perches
-- strict thread matches above, loose matches on Subject: below --
2012-05-04 0:22 Jim Cromie
2012-05-03 17:57 [PATCH 1/3] params.c: fix Smack complaint about parse_args Jim Cromie
2012-05-03 17:57 ` [PATCH 2/3] params: replace printk(KERN_<LVL>...) with pr_<lvl>(...) Jim Cromie
2012-05-03 18:50 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox