* [PATCH] kernel:module Fix coding style errors and warnings.
@ 2014-10-16 9:36 Ionut Alexa
2014-10-16 9:38 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Ionut Alexa @ 2014-10-16 9:36 UTC (permalink / raw)
To: rusty; +Cc: linux-kernel, Ionut Alexa
Fixed codin style errors and warnings. Changes printk with
print_debug/warn. Changed seq_printf to seq_puts.
Signed-off-by: Ionut Alexa <ionut.m.alexa@gmail.com>
---
kernel/module.c | 53 +++++++++++++++++++++++++----------------------------
1 file changed, 25 insertions(+), 28 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 65586ff..bd159b5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -110,7 +110,7 @@ struct list_head *kdb_modules = &modules; /* kdb needs the list of modules */
#ifdef CONFIG_MODULE_SIG_FORCE
static bool sig_enforce = true;
#else
-static bool sig_enforce = false;
+static bool sig_enforce; /* by default set to false */
static int param_set_bool_enable_only(const char *val,
const struct kernel_param *kp)
@@ -156,15 +156,15 @@ static BLOCKING_NOTIFIER_HEAD(module_notify_list);
/* Bounds of module allocation, for speeding __module_address.
* Protected by module_mutex. */
-static unsigned long module_addr_min = -1UL, module_addr_max = 0;
+static unsigned long module_addr_min = -1UL, module_addr_max; /* addr_max=0 */
-int register_module_notifier(struct notifier_block * nb)
+int register_module_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_register(&module_notify_list, nb);
}
EXPORT_SYMBOL(register_module_notifier);
-int unregister_module_notifier(struct notifier_block * nb)
+int unregister_module_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_unregister(&module_notify_list, nb);
}
@@ -740,8 +740,7 @@ static inline int try_force_unload(unsigned int flags)
}
#endif /* CONFIG_MODULE_FORCE_UNLOAD */
-struct stopref
-{
+struct stopref{
struct module *mod;
int flags;
int *forced;
@@ -878,7 +877,7 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
seq_printf(m, " %lu ", module_refcount(mod));
/* Always include a trailing , so userspace can differentiate
- between this and the old multi-field proc format. */
+ * between this and the old multi-field proc format. */
list_for_each_entry(use, &mod->source_list, source_list) {
printed_something = 1;
seq_printf(m, "%s,", use->source->name);
@@ -886,11 +885,11 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
if (mod->init != NULL && mod->exit == NULL) {
printed_something = 1;
- seq_printf(m, "[permanent],");
+ seq_puts(m, "[permanent],");
}
if (!printed_something)
- seq_printf(m, "-");
+ seq_puts(m, "-");
}
void __symbol_put(const char *symbol)
@@ -978,7 +977,7 @@ EXPORT_SYMBOL(module_put);
static inline void print_unload_info(struct seq_file *m, struct module *mod)
{
/* We don't know the usage count, or what modules are using. */
- seq_printf(m, " - -");
+ seq_puts(m, " - -");
}
static inline void module_unload_free(struct module *mod)
@@ -1131,7 +1130,7 @@ static unsigned long maybe_relocated(unsigned long crc,
static int check_version(Elf_Shdr *sechdrs,
unsigned int versindex,
const char *symname,
- struct module *mod,
+ struct module *mod,
const unsigned long *crc,
const struct module *crc_owner)
{
@@ -1165,7 +1164,7 @@ static int check_version(Elf_Shdr *sechdrs,
return 0;
bad_version:
- printk("%s: disagrees about version of symbol %s\n",
+ pr_warn("%s: disagrees about version of symbol %s\n",
mod->name, symname);
return 0;
}
@@ -1200,7 +1199,7 @@ static inline int same_magic(const char *amagic, const char *bmagic,
static inline int check_version(Elf_Shdr *sechdrs,
unsigned int versindex,
const char *symname,
- struct module *mod,
+ struct module *mod,
const unsigned long *crc,
const struct module *crc_owner)
{
@@ -1288,15 +1287,13 @@ static inline bool sect_empty(const Elf_Shdr *sect)
return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
}
-struct module_sect_attr
-{
+struct module_sect_attr {
struct module_attribute mattr;
char *name;
unsigned long address;
};
-struct module_sect_attrs
-{
+struct module_sect_attrs {
struct attribute_group grp;
unsigned int nsections;
struct module_sect_attr attrs[0];
@@ -1550,7 +1547,7 @@ static int module_add_modinfo_attrs(struct module *mod)
(attr->test && attr->test(mod))) {
memcpy(temp_attr, attr, sizeof(*temp_attr));
sysfs_attr_init(&temp_attr->attr);
- error = sysfs_create_file(&mod->mkobj.kobj,&temp_attr->attr);
+ error = sysfs_create_file(&mod->mkobj.kobj, &temp_attr->attr);
++temp_attr;
}
}
@@ -1566,7 +1563,7 @@ static void module_remove_modinfo_attrs(struct module *mod)
/* pick a field to test for end of list */
if (!attr->attr.name)
break;
- sysfs_remove_file(&mod->mkobj.kobj,&attr->attr);
+ sysfs_remove_file(&mod->mkobj.kobj, &attr->attr);
if (attr->free)
attr->free(mod);
}
@@ -1953,7 +1950,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
/* We compiled with -fno-common. These are not
supposed to happen. */
pr_debug("Common symbol: %s\n", name);
- printk("%s: please compile with -fno-common\n",
+ pr_debug("%s: please compile with -fno-common\n",
mod->name);
ret = -ENOEXEC;
break;
@@ -2257,7 +2254,7 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
}
static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
- unsigned int shnum)
+ unsigned int shnum)
{
const Elf_Shdr *sec;
@@ -2733,7 +2730,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
* This shouldn't happen with same compiler and binutils
* building all parts of the module.
*/
- printk(KERN_WARNING "%s: has both .ctors and .init_array.\n",
+ pr_warn("%s: has both .ctors and .init_array.\n",
mod->name);
return -EINVAL;
}
@@ -3022,7 +3019,7 @@ static int do_init_module(struct module *mod)
ret = do_one_initcall(mod->init);
if (ret < 0) {
/* Init routine failed: abort. Try to protect us from
- buggy refcounters. */
+ * buggy refcounters. */
mod->state = MODULE_STATE_GOING;
synchronize_sched();
module_put(mod);
@@ -3174,7 +3171,7 @@ out:
static int unknown_module_param_cb(char *param, char *val, const char *modname)
{
- /* Check for magic 'dyndbg' arg */
+ /* Check for magic 'dyndbg' arg */
int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
if (ret != 0)
pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
@@ -3657,8 +3654,8 @@ static int m_show(struct seq_file *m, void *p)
/* Informative for users. */
seq_printf(m, " %s",
- mod->state == MODULE_STATE_GOING ? "Unloading":
- mod->state == MODULE_STATE_COMING ? "Loading":
+ mod->state == MODULE_STATE_GOING ? "Unloading" :
+ mod->state == MODULE_STATE_COMING ? "Loading" :
"Live");
/* Used by oprofile and other similar tools. */
seq_printf(m, " 0x%pK", mod->module_core);
@@ -3667,7 +3664,7 @@ static int m_show(struct seq_file *m, void *p)
if (mod->taints)
seq_printf(m, " %s", module_flags(mod, buf));
- seq_printf(m, "\n");
+ seq_puts(m, "\n");
return 0;
}
@@ -3816,7 +3813,7 @@ void print_modules(void)
struct module *mod;
char buf[8];
- printk(KERN_DEFAULT "Modules linked in:");
+ pr_warn("Modules linked in:");
/* Most callers should already have preempt disabled, but make sure */
preempt_disable();
list_for_each_entry_rcu(mod, &modules, list) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kernel:module Fix coding style errors and warnings.
2014-10-16 9:36 [PATCH] kernel:module Fix coding style errors and warnings Ionut Alexa
@ 2014-10-16 9:38 ` Joe Perches
2014-10-16 12:57 ` Sudip Mukherjee
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2014-10-16 9:38 UTC (permalink / raw)
To: Ionut Alexa; +Cc: rusty, linux-kernel
On Thu, 2014-10-16 at 12:36 +0300, Ionut Alexa wrote:
> Fixed codin style errors and warnings. Changes printk with
> print_debug/warn. Changed seq_printf to seq_puts.
If you are going to do style cleanups, please make
sure you check your patches with scripts/checkpatch.pl
before sending them.
WARNING: missing space after struct definition
#104: FILE: kernel/module.c:743:
+struct stopref{
WARNING: line over 80 characters
#190: FILE: kernel/module.c:1550:
+ error = sysfs_create_file(&mod->mkobj.kobj, &temp_attr->attr);
total: 0 errors, 2 warnings, 187 lines checked
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kernel:module Fix coding style errors and warnings.
2014-10-16 9:38 ` Joe Perches
@ 2014-10-16 12:57 ` Sudip Mukherjee
2014-10-16 14:46 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Sudip Mukherjee @ 2014-10-16 12:57 UTC (permalink / raw)
To: Joe Perches; +Cc: Ionut Alexa, rusty, linux-kernel
On Thu, Oct 16, 2014 at 02:38:53AM -0700, Joe Perches wrote:
> On Thu, 2014-10-16 at 12:36 +0300, Ionut Alexa wrote:
> > Fixed codin style errors and warnings. Changes printk with
> > print_debug/warn. Changed seq_printf to seq_puts.
>
> If you are going to do style cleanups, please make
> sure you check your patches with scripts/checkpatch.pl
> before sending them.
>
> WARNING: missing space after struct definition
> #104: FILE: kernel/module.c:743:
> +struct stopref{
>
> WARNING: line over 80 characters
> #190: FILE: kernel/module.c:1550:
> + error = sysfs_create_file(&mod->mkobj.kobj, &temp_attr->attr);
>
> total: 0 errors, 2 warnings, 187 lines checked
>
hi joe,
should we be doing style cleanups in the kernel files ?
previously i saw comments discouraging it as it pollutes the git blame.
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] kernel:module Fix coding style errors and warnings.
2014-10-16 12:57 ` Sudip Mukherjee
@ 2014-10-16 14:46 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2014-10-16 14:46 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Ionut Alexa, rusty, linux-kernel
On Thu, 2014-10-16 at 18:27 +0530, Sudip Mukherjee wrote:
> On Thu, Oct 16, 2014 at 02:38:53AM -0700, Joe Perches wrote:
> > On Thu, 2014-10-16 at 12:36 +0300, Ionut Alexa wrote:
> > > Fixed codin style errors and warnings. Changes printk with
> > > print_debug/warn. Changed seq_printf to seq_puts.
> > If you are going to do style cleanups, please make
> > sure you check your patches with scripts/checkpatch.pl
> > before sending them.
[]
> should we be doing style cleanups in the kernel files ?
Generally, no.
> previously i saw comments discouraging it as it pollutes the git blame.
Also generally, style only patches are discouraged
but can be done as part of a series when enhancements,
bug-fixes or optimizations are done.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] kernel:module Fix coding style errors and warnings.
@ 2014-10-16 10:35 Ionut Alexa
2014-10-16 23:06 ` Rusty Russell
0 siblings, 1 reply; 8+ messages in thread
From: Ionut Alexa @ 2014-10-16 10:35 UTC (permalink / raw)
To: rusty; +Cc: linux-kernel, Ionut Alexa
Fixed codin style errors and warnings. Changes printk with
print_debug/warn. Changed seq_printf to seq_puts.
Signed-off-by: Ionut Alexa <ionut.m.alexa@gmail.com>
---
kernel/module.c | 54 ++++++++++++++++++++++++++----------------------------
1 file changed, 26 insertions(+), 28 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 65586ff..9fb16f7 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -110,7 +110,7 @@ struct list_head *kdb_modules = &modules; /* kdb needs the list of modules */
#ifdef CONFIG_MODULE_SIG_FORCE
static bool sig_enforce = true;
#else
-static bool sig_enforce = false;
+static bool sig_enforce; /* by default set to false */
static int param_set_bool_enable_only(const char *val,
const struct kernel_param *kp)
@@ -156,15 +156,15 @@ static BLOCKING_NOTIFIER_HEAD(module_notify_list);
/* Bounds of module allocation, for speeding __module_address.
* Protected by module_mutex. */
-static unsigned long module_addr_min = -1UL, module_addr_max = 0;
+static unsigned long module_addr_min = -1UL, module_addr_max; /* addr_max=0 */
-int register_module_notifier(struct notifier_block * nb)
+int register_module_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_register(&module_notify_list, nb);
}
EXPORT_SYMBOL(register_module_notifier);
-int unregister_module_notifier(struct notifier_block * nb)
+int unregister_module_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_unregister(&module_notify_list, nb);
}
@@ -740,8 +740,7 @@ static inline int try_force_unload(unsigned int flags)
}
#endif /* CONFIG_MODULE_FORCE_UNLOAD */
-struct stopref
-{
+struct stopref {
struct module *mod;
int flags;
int *forced;
@@ -878,7 +877,7 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
seq_printf(m, " %lu ", module_refcount(mod));
/* Always include a trailing , so userspace can differentiate
- between this and the old multi-field proc format. */
+ * between this and the old multi-field proc format. */
list_for_each_entry(use, &mod->source_list, source_list) {
printed_something = 1;
seq_printf(m, "%s,", use->source->name);
@@ -886,11 +885,11 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
if (mod->init != NULL && mod->exit == NULL) {
printed_something = 1;
- seq_printf(m, "[permanent],");
+ seq_puts(m, "[permanent],");
}
if (!printed_something)
- seq_printf(m, "-");
+ seq_puts(m, "-");
}
void __symbol_put(const char *symbol)
@@ -978,7 +977,7 @@ EXPORT_SYMBOL(module_put);
static inline void print_unload_info(struct seq_file *m, struct module *mod)
{
/* We don't know the usage count, or what modules are using. */
- seq_printf(m, " - -");
+ seq_puts(m, " - -");
}
static inline void module_unload_free(struct module *mod)
@@ -1131,7 +1130,7 @@ static unsigned long maybe_relocated(unsigned long crc,
static int check_version(Elf_Shdr *sechdrs,
unsigned int versindex,
const char *symname,
- struct module *mod,
+ struct module *mod,
const unsigned long *crc,
const struct module *crc_owner)
{
@@ -1165,7 +1164,7 @@ static int check_version(Elf_Shdr *sechdrs,
return 0;
bad_version:
- printk("%s: disagrees about version of symbol %s\n",
+ pr_warn("%s: disagrees about version of symbol %s\n",
mod->name, symname);
return 0;
}
@@ -1200,7 +1199,7 @@ static inline int same_magic(const char *amagic, const char *bmagic,
static inline int check_version(Elf_Shdr *sechdrs,
unsigned int versindex,
const char *symname,
- struct module *mod,
+ struct module *mod,
const unsigned long *crc,
const struct module *crc_owner)
{
@@ -1288,15 +1287,13 @@ static inline bool sect_empty(const Elf_Shdr *sect)
return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
}
-struct module_sect_attr
-{
+struct module_sect_attr {
struct module_attribute mattr;
char *name;
unsigned long address;
};
-struct module_sect_attrs
-{
+struct module_sect_attrs {
struct attribute_group grp;
unsigned int nsections;
struct module_sect_attr attrs[0];
@@ -1550,7 +1547,8 @@ static int module_add_modinfo_attrs(struct module *mod)
(attr->test && attr->test(mod))) {
memcpy(temp_attr, attr, sizeof(*temp_attr));
sysfs_attr_init(&temp_attr->attr);
- error = sysfs_create_file(&mod->mkobj.kobj,&temp_attr->attr);
+ error = sysfs_create_file(&mod->mkobj.kobj,
+ &temp_attr->attr);
++temp_attr;
}
}
@@ -1566,7 +1564,7 @@ static void module_remove_modinfo_attrs(struct module *mod)
/* pick a field to test for end of list */
if (!attr->attr.name)
break;
- sysfs_remove_file(&mod->mkobj.kobj,&attr->attr);
+ sysfs_remove_file(&mod->mkobj.kobj, &attr->attr);
if (attr->free)
attr->free(mod);
}
@@ -1953,7 +1951,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
/* We compiled with -fno-common. These are not
supposed to happen. */
pr_debug("Common symbol: %s\n", name);
- printk("%s: please compile with -fno-common\n",
+ pr_debug("%s: please compile with -fno-common\n",
mod->name);
ret = -ENOEXEC;
break;
@@ -2257,7 +2255,7 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
}
static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
- unsigned int shnum)
+ unsigned int shnum)
{
const Elf_Shdr *sec;
@@ -2733,7 +2731,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
* This shouldn't happen with same compiler and binutils
* building all parts of the module.
*/
- printk(KERN_WARNING "%s: has both .ctors and .init_array.\n",
+ pr_warn("%s: has both .ctors and .init_array.\n",
mod->name);
return -EINVAL;
}
@@ -3022,7 +3020,7 @@ static int do_init_module(struct module *mod)
ret = do_one_initcall(mod->init);
if (ret < 0) {
/* Init routine failed: abort. Try to protect us from
- buggy refcounters. */
+ * buggy refcounters. */
mod->state = MODULE_STATE_GOING;
synchronize_sched();
module_put(mod);
@@ -3174,7 +3172,7 @@ out:
static int unknown_module_param_cb(char *param, char *val, const char *modname)
{
- /* Check for magic 'dyndbg' arg */
+ /* Check for magic 'dyndbg' arg */
int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
if (ret != 0)
pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
@@ -3657,8 +3655,8 @@ static int m_show(struct seq_file *m, void *p)
/* Informative for users. */
seq_printf(m, " %s",
- mod->state == MODULE_STATE_GOING ? "Unloading":
- mod->state == MODULE_STATE_COMING ? "Loading":
+ mod->state == MODULE_STATE_GOING ? "Unloading" :
+ mod->state == MODULE_STATE_COMING ? "Loading" :
"Live");
/* Used by oprofile and other similar tools. */
seq_printf(m, " 0x%pK", mod->module_core);
@@ -3667,7 +3665,7 @@ static int m_show(struct seq_file *m, void *p)
if (mod->taints)
seq_printf(m, " %s", module_flags(mod, buf));
- seq_printf(m, "\n");
+ seq_puts(m, "\n");
return 0;
}
@@ -3816,7 +3814,7 @@ void print_modules(void)
struct module *mod;
char buf[8];
- printk(KERN_DEFAULT "Modules linked in:");
+ pr_warn("Modules linked in:");
/* Most callers should already have preempt disabled, but make sure */
preempt_disable();
list_for_each_entry_rcu(mod, &modules, list) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kernel:module Fix coding style errors and warnings.
2014-10-16 10:35 Ionut Alexa
@ 2014-10-16 23:06 ` Rusty Russell
0 siblings, 0 replies; 8+ messages in thread
From: Rusty Russell @ 2014-10-16 23:06 UTC (permalink / raw)
To: Ionut Alexa; +Cc: linux-kernel, Ionut Alexa
Ionut Alexa <ionut.m.alexa@gmail.com> writes:
> Fixed codin style errors and warnings. Changes printk with
> print_debug/warn. Changed seq_printf to seq_puts.
>
> Signed-off-by: Ionut Alexa <ionut.m.alexa@gmail.com>
Hi Ionut,
Please drop the following changes:
> @@ -110,7 +110,7 @@ struct list_head *kdb_modules = &modules; /* kdb needs the list of modules */
> #ifdef CONFIG_MODULE_SIG_FORCE
> static bool sig_enforce = true;
> #else
> -static bool sig_enforce = false;
> +static bool sig_enforce; /* by default set to false */
>
> static int param_set_bool_enable_only(const char *val,
> const struct kernel_param *kp)
> @@ -156,15 +156,15 @@ static BLOCKING_NOTIFIER_HEAD(module_notify_list);
>
> /* Bounds of module allocation, for speeding __module_address.
> * Protected by module_mutex. */
> -static unsigned long module_addr_min = -1UL, module_addr_max = 0;
> +static unsigned long module_addr_min = -1UL, module_addr_max; /* addr_max=0 */
I think the explicit initializers are clearer. Gcc realizes they're
zero and puts them in bss anyway, so there's no size cost.
> -int register_module_notifier(struct notifier_block * nb)
> +int register_module_notifier(struct notifier_block *nb)
> {
> return blocking_notifier_chain_register(&module_notify_list, nb);
> }
> EXPORT_SYMBOL(register_module_notifier);
>
> -int unregister_module_notifier(struct notifier_block * nb)
> +int unregister_module_notifier(struct notifier_block *nb)
> {
> return blocking_notifier_chain_unregister(&module_notify_list, nb);
> }
> @@ -740,8 +740,7 @@ static inline int try_force_unload(unsigned int flags)
> }
> #endif /* CONFIG_MODULE_FORCE_UNLOAD */
>
> -struct stopref
> -{
> +struct stopref {
> struct module *mod;
> int flags;
> int *forced;
These are fine.
> @@ -878,7 +877,7 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
> seq_printf(m, " %lu ", module_refcount(mod));
>
> /* Always include a trailing , so userspace can differentiate
> - between this and the old multi-field proc format. */
> + * between this and the old multi-field proc format. */
> list_for_each_entry(use, &mod->source_list, source_list) {
> printed_something = 1;
> seq_printf(m, "%s,", use->source->name);
Actually, kernel style for multi-line comments, like it or not, is:
/*
* Always include a trailing , so userspace can differentiate
* between this and the old multi-field proc format.
*/
> @@ -1953,7 +1951,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
> /* We compiled with -fno-common. These are not
> supposed to happen. */
> pr_debug("Common symbol: %s\n", name);
> - printk("%s: please compile with -fno-common\n",
> + pr_debug("%s: please compile with -fno-common\n",
> mod->name);
> ret = -ENOEXEC;
> break;
Please change it to pr_warn rather than pr_debug!
> @@ -3022,7 +3020,7 @@ static int do_init_module(struct module *mod)
> ret = do_one_initcall(mod->init);
> if (ret < 0) {
> /* Init routine failed: abort. Try to protect us from
> - buggy refcounters. */
> + * buggy refcounters. */
> mod->state = MODULE_STATE_GOING;
> synchronize_sched();
> module_put(mod);
> @@ -3174,7 +3172,7 @@ out:
Comment style here, too.
> @@ -3816,7 +3814,7 @@ void print_modules(void)
> struct module *mod;
> char buf[8];
>
> - printk(KERN_DEFAULT "Modules linked in:");
> + pr_warn("Modules linked in:");
This is not the same as KERN_DEFAULT; is it correct?
Thanks,
Rusty.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] kernel:module Fix coding style errors and warnings.
@ 2014-10-17 8:32 Ionut Alexa
2014-10-19 23:57 ` Rusty Russell
0 siblings, 1 reply; 8+ messages in thread
From: Ionut Alexa @ 2014-10-17 8:32 UTC (permalink / raw)
To: rusty; +Cc: linux-kernel, Ionut Alexa
Fixed codin style errors and warnings. Changes printk with
print_debug/warn. Changed seq_printf to seq_puts.
Signed-off-by: Ionut Alexa <ionut.m.alexa@gmail.com>
---
kernel/module.c | 58 ++++++++++++++++++++++++++++---------------------------
1 file changed, 30 insertions(+), 28 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 65586ff..b19ea41 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -158,13 +158,13 @@ static BLOCKING_NOTIFIER_HEAD(module_notify_list);
* Protected by module_mutex. */
static unsigned long module_addr_min = -1UL, module_addr_max = 0;
-int register_module_notifier(struct notifier_block * nb)
+int register_module_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_register(&module_notify_list, nb);
}
EXPORT_SYMBOL(register_module_notifier);
-int unregister_module_notifier(struct notifier_block * nb)
+int unregister_module_notifier(struct notifier_block *nb)
{
return blocking_notifier_chain_unregister(&module_notify_list, nb);
}
@@ -740,8 +740,7 @@ static inline int try_force_unload(unsigned int flags)
}
#endif /* CONFIG_MODULE_FORCE_UNLOAD */
-struct stopref
-{
+struct stopref {
struct module *mod;
int flags;
int *forced;
@@ -877,8 +876,10 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
seq_printf(m, " %lu ", module_refcount(mod));
- /* Always include a trailing , so userspace can differentiate
- between this and the old multi-field proc format. */
+ /*
+ * Always include a trailing , so userspace can differentiate
+ * between this and the old multi-field proc format.
+ */
list_for_each_entry(use, &mod->source_list, source_list) {
printed_something = 1;
seq_printf(m, "%s,", use->source->name);
@@ -886,11 +887,11 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
if (mod->init != NULL && mod->exit == NULL) {
printed_something = 1;
- seq_printf(m, "[permanent],");
+ seq_puts(m, "[permanent],");
}
if (!printed_something)
- seq_printf(m, "-");
+ seq_puts(m, "-");
}
void __symbol_put(const char *symbol)
@@ -978,7 +979,7 @@ EXPORT_SYMBOL(module_put);
static inline void print_unload_info(struct seq_file *m, struct module *mod)
{
/* We don't know the usage count, or what modules are using. */
- seq_printf(m, " - -");
+ seq_puts(m, " - -");
}
static inline void module_unload_free(struct module *mod)
@@ -1131,7 +1132,7 @@ static unsigned long maybe_relocated(unsigned long crc,
static int check_version(Elf_Shdr *sechdrs,
unsigned int versindex,
const char *symname,
- struct module *mod,
+ struct module *mod,
const unsigned long *crc,
const struct module *crc_owner)
{
@@ -1165,7 +1166,7 @@ static int check_version(Elf_Shdr *sechdrs,
return 0;
bad_version:
- printk("%s: disagrees about version of symbol %s\n",
+ pr_warn("%s: disagrees about version of symbol %s\n",
mod->name, symname);
return 0;
}
@@ -1200,7 +1201,7 @@ static inline int same_magic(const char *amagic, const char *bmagic,
static inline int check_version(Elf_Shdr *sechdrs,
unsigned int versindex,
const char *symname,
- struct module *mod,
+ struct module *mod,
const unsigned long *crc,
const struct module *crc_owner)
{
@@ -1288,15 +1289,13 @@ static inline bool sect_empty(const Elf_Shdr *sect)
return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
}
-struct module_sect_attr
-{
+struct module_sect_attr {
struct module_attribute mattr;
char *name;
unsigned long address;
};
-struct module_sect_attrs
-{
+struct module_sect_attrs {
struct attribute_group grp;
unsigned int nsections;
struct module_sect_attr attrs[0];
@@ -1550,7 +1549,8 @@ static int module_add_modinfo_attrs(struct module *mod)
(attr->test && attr->test(mod))) {
memcpy(temp_attr, attr, sizeof(*temp_attr));
sysfs_attr_init(&temp_attr->attr);
- error = sysfs_create_file(&mod->mkobj.kobj,&temp_attr->attr);
+ error = sysfs_create_file(&mod->mkobj.kobj,
+ &temp_attr->attr);
++temp_attr;
}
}
@@ -1566,7 +1566,7 @@ static void module_remove_modinfo_attrs(struct module *mod)
/* pick a field to test for end of list */
if (!attr->attr.name)
break;
- sysfs_remove_file(&mod->mkobj.kobj,&attr->attr);
+ sysfs_remove_file(&mod->mkobj.kobj, &attr->attr);
if (attr->free)
attr->free(mod);
}
@@ -1953,7 +1953,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
/* We compiled with -fno-common. These are not
supposed to happen. */
pr_debug("Common symbol: %s\n", name);
- printk("%s: please compile with -fno-common\n",
+ pr_warn("%s: please compile with -fno-common\n",
mod->name);
ret = -ENOEXEC;
break;
@@ -2257,7 +2257,7 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
}
static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
- unsigned int shnum)
+ unsigned int shnum)
{
const Elf_Shdr *sec;
@@ -2733,7 +2733,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
* This shouldn't happen with same compiler and binutils
* building all parts of the module.
*/
- printk(KERN_WARNING "%s: has both .ctors and .init_array.\n",
+ pr_warn("%s: has both .ctors and .init_array.\n",
mod->name);
return -EINVAL;
}
@@ -3021,8 +3021,10 @@ static int do_init_module(struct module *mod)
if (mod->init != NULL)
ret = do_one_initcall(mod->init);
if (ret < 0) {
- /* Init routine failed: abort. Try to protect us from
- buggy refcounters. */
+ /*
+ * Init routine failed: abort. Try to protect us from
+ * buggy refcounters.
+ */
mod->state = MODULE_STATE_GOING;
synchronize_sched();
module_put(mod);
@@ -3174,7 +3176,7 @@ out:
static int unknown_module_param_cb(char *param, char *val, const char *modname)
{
- /* Check for magic 'dyndbg' arg */
+ /* Check for magic 'dyndbg' arg */
int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
if (ret != 0)
pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
@@ -3657,8 +3659,8 @@ static int m_show(struct seq_file *m, void *p)
/* Informative for users. */
seq_printf(m, " %s",
- mod->state == MODULE_STATE_GOING ? "Unloading":
- mod->state == MODULE_STATE_COMING ? "Loading":
+ mod->state == MODULE_STATE_GOING ? "Unloading" :
+ mod->state == MODULE_STATE_COMING ? "Loading" :
"Live");
/* Used by oprofile and other similar tools. */
seq_printf(m, " 0x%pK", mod->module_core);
@@ -3667,7 +3669,7 @@ static int m_show(struct seq_file *m, void *p)
if (mod->taints)
seq_printf(m, " %s", module_flags(mod, buf));
- seq_printf(m, "\n");
+ seq_puts(m, "\n");
return 0;
}
@@ -3816,7 +3818,7 @@ void print_modules(void)
struct module *mod;
char buf[8];
- printk(KERN_DEFAULT "Modules linked in:");
+ pr_warn("Modules linked in:");
/* Most callers should already have preempt disabled, but make sure */
preempt_disable();
list_for_each_entry_rcu(mod, &modules, list) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] kernel:module Fix coding style errors and warnings.
2014-10-17 8:32 Ionut Alexa
@ 2014-10-19 23:57 ` Rusty Russell
0 siblings, 0 replies; 8+ messages in thread
From: Rusty Russell @ 2014-10-19 23:57 UTC (permalink / raw)
To: Ionut Alexa; +Cc: linux-kernel, Ionut Alexa
Ionut Alexa <ionut.m.alexa@gmail.com> writes:
> Fixed codin style errors and warnings. Changes printk with
> print_debug/warn. Changed seq_printf to seq_puts.
>
> Signed-off-by: Ionut Alexa <ionut.m.alexa@gmail.com>
Removed the last hunk, and applied.
Thanks!
Rusty.
> ---
> kernel/module.c | 58 ++++++++++++++++++++++++++++---------------------------
> 1 file changed, 30 insertions(+), 28 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 65586ff..b19ea41 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -158,13 +158,13 @@ static BLOCKING_NOTIFIER_HEAD(module_notify_list);
> * Protected by module_mutex. */
> static unsigned long module_addr_min = -1UL, module_addr_max = 0;
>
> -int register_module_notifier(struct notifier_block * nb)
> +int register_module_notifier(struct notifier_block *nb)
> {
> return blocking_notifier_chain_register(&module_notify_list, nb);
> }
> EXPORT_SYMBOL(register_module_notifier);
>
> -int unregister_module_notifier(struct notifier_block * nb)
> +int unregister_module_notifier(struct notifier_block *nb)
> {
> return blocking_notifier_chain_unregister(&module_notify_list, nb);
> }
> @@ -740,8 +740,7 @@ static inline int try_force_unload(unsigned int flags)
> }
> #endif /* CONFIG_MODULE_FORCE_UNLOAD */
>
> -struct stopref
> -{
> +struct stopref {
> struct module *mod;
> int flags;
> int *forced;
> @@ -877,8 +876,10 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
>
> seq_printf(m, " %lu ", module_refcount(mod));
>
> - /* Always include a trailing , so userspace can differentiate
> - between this and the old multi-field proc format. */
> + /*
> + * Always include a trailing , so userspace can differentiate
> + * between this and the old multi-field proc format.
> + */
> list_for_each_entry(use, &mod->source_list, source_list) {
> printed_something = 1;
> seq_printf(m, "%s,", use->source->name);
> @@ -886,11 +887,11 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
>
> if (mod->init != NULL && mod->exit == NULL) {
> printed_something = 1;
> - seq_printf(m, "[permanent],");
> + seq_puts(m, "[permanent],");
> }
>
> if (!printed_something)
> - seq_printf(m, "-");
> + seq_puts(m, "-");
> }
>
> void __symbol_put(const char *symbol)
> @@ -978,7 +979,7 @@ EXPORT_SYMBOL(module_put);
> static inline void print_unload_info(struct seq_file *m, struct module *mod)
> {
> /* We don't know the usage count, or what modules are using. */
> - seq_printf(m, " - -");
> + seq_puts(m, " - -");
> }
>
> static inline void module_unload_free(struct module *mod)
> @@ -1131,7 +1132,7 @@ static unsigned long maybe_relocated(unsigned long crc,
> static int check_version(Elf_Shdr *sechdrs,
> unsigned int versindex,
> const char *symname,
> - struct module *mod,
> + struct module *mod,
> const unsigned long *crc,
> const struct module *crc_owner)
> {
> @@ -1165,7 +1166,7 @@ static int check_version(Elf_Shdr *sechdrs,
> return 0;
>
> bad_version:
> - printk("%s: disagrees about version of symbol %s\n",
> + pr_warn("%s: disagrees about version of symbol %s\n",
> mod->name, symname);
> return 0;
> }
> @@ -1200,7 +1201,7 @@ static inline int same_magic(const char *amagic, const char *bmagic,
> static inline int check_version(Elf_Shdr *sechdrs,
> unsigned int versindex,
> const char *symname,
> - struct module *mod,
> + struct module *mod,
> const unsigned long *crc,
> const struct module *crc_owner)
> {
> @@ -1288,15 +1289,13 @@ static inline bool sect_empty(const Elf_Shdr *sect)
> return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0;
> }
>
> -struct module_sect_attr
> -{
> +struct module_sect_attr {
> struct module_attribute mattr;
> char *name;
> unsigned long address;
> };
>
> -struct module_sect_attrs
> -{
> +struct module_sect_attrs {
> struct attribute_group grp;
> unsigned int nsections;
> struct module_sect_attr attrs[0];
> @@ -1550,7 +1549,8 @@ static int module_add_modinfo_attrs(struct module *mod)
> (attr->test && attr->test(mod))) {
> memcpy(temp_attr, attr, sizeof(*temp_attr));
> sysfs_attr_init(&temp_attr->attr);
> - error = sysfs_create_file(&mod->mkobj.kobj,&temp_attr->attr);
> + error = sysfs_create_file(&mod->mkobj.kobj,
> + &temp_attr->attr);
> ++temp_attr;
> }
> }
> @@ -1566,7 +1566,7 @@ static void module_remove_modinfo_attrs(struct module *mod)
> /* pick a field to test for end of list */
> if (!attr->attr.name)
> break;
> - sysfs_remove_file(&mod->mkobj.kobj,&attr->attr);
> + sysfs_remove_file(&mod->mkobj.kobj, &attr->attr);
> if (attr->free)
> attr->free(mod);
> }
> @@ -1953,7 +1953,7 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
> /* We compiled with -fno-common. These are not
> supposed to happen. */
> pr_debug("Common symbol: %s\n", name);
> - printk("%s: please compile with -fno-common\n",
> + pr_warn("%s: please compile with -fno-common\n",
> mod->name);
> ret = -ENOEXEC;
> break;
> @@ -2257,7 +2257,7 @@ static char elf_type(const Elf_Sym *sym, const struct load_info *info)
> }
>
> static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
> - unsigned int shnum)
> + unsigned int shnum)
> {
> const Elf_Shdr *sec;
>
> @@ -2733,7 +2733,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
> * This shouldn't happen with same compiler and binutils
> * building all parts of the module.
> */
> - printk(KERN_WARNING "%s: has both .ctors and .init_array.\n",
> + pr_warn("%s: has both .ctors and .init_array.\n",
> mod->name);
> return -EINVAL;
> }
> @@ -3021,8 +3021,10 @@ static int do_init_module(struct module *mod)
> if (mod->init != NULL)
> ret = do_one_initcall(mod->init);
> if (ret < 0) {
> - /* Init routine failed: abort. Try to protect us from
> - buggy refcounters. */
> + /*
> + * Init routine failed: abort. Try to protect us from
> + * buggy refcounters.
> + */
> mod->state = MODULE_STATE_GOING;
> synchronize_sched();
> module_put(mod);
> @@ -3174,7 +3176,7 @@ out:
>
> static int unknown_module_param_cb(char *param, char *val, const char *modname)
> {
> - /* Check for magic 'dyndbg' arg */
> + /* Check for magic 'dyndbg' arg */
> int ret = ddebug_dyndbg_module_param_cb(param, val, modname);
> if (ret != 0)
> pr_warn("%s: unknown parameter '%s' ignored\n", modname, param);
> @@ -3657,8 +3659,8 @@ static int m_show(struct seq_file *m, void *p)
>
> /* Informative for users. */
> seq_printf(m, " %s",
> - mod->state == MODULE_STATE_GOING ? "Unloading":
> - mod->state == MODULE_STATE_COMING ? "Loading":
> + mod->state == MODULE_STATE_GOING ? "Unloading" :
> + mod->state == MODULE_STATE_COMING ? "Loading" :
> "Live");
> /* Used by oprofile and other similar tools. */
> seq_printf(m, " 0x%pK", mod->module_core);
> @@ -3667,7 +3669,7 @@ static int m_show(struct seq_file *m, void *p)
> if (mod->taints)
> seq_printf(m, " %s", module_flags(mod, buf));
>
> - seq_printf(m, "\n");
> + seq_puts(m, "\n");
> return 0;
> }
>
> @@ -3816,7 +3818,7 @@ void print_modules(void)
> struct module *mod;
> char buf[8];
>
> - printk(KERN_DEFAULT "Modules linked in:");
> + pr_warn("Modules linked in:");
> /* Most callers should already have preempt disabled, but make sure */
> preempt_disable();
> list_for_each_entry_rcu(mod, &modules, list) {
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-10-20 0:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-16 9:36 [PATCH] kernel:module Fix coding style errors and warnings Ionut Alexa
2014-10-16 9:38 ` Joe Perches
2014-10-16 12:57 ` Sudip Mukherjee
2014-10-16 14:46 ` Joe Perches
2014-10-16 10:35 Ionut Alexa
2014-10-16 23:06 ` Rusty Russell
2014-10-17 8:32 Ionut Alexa
2014-10-19 23:57 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).