From: Alexander Beregalov <[email protected]>
kernel/module.c: In function 'load_module':
kernel/module.c:1839: warning: unused variable 'value'
kernel/module.c:1838: warning: unused variable 'iter'
Signed-off-by: Alexander Beregalov <[email protected]>
---
kernel/module.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 1721320..5a1258e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1835,8 +1835,10 @@ static struct module *load_module(void __user *umod,
unsigned int markersindex;
unsigned int markersstringsindex;
unsigned int verboseindex;
+#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
struct mod_debug *iter;
unsigned long value;
+#endif
unsigned int tracepointsindex;
unsigned int tracepointsstringsindex;
struct module *mod;
On Wed, Aug 20, 2008 at 06:13:29PM +0400, Alexander Beregalov wrote:
> From: Alexander Beregalov <[email protected]>
>
> kernel/module.c: In function 'load_module':
> kernel/module.c:1839: warning: unused variable 'value'
> kernel/module.c:1838: warning: unused variable 'iter'
>
>
> Signed-off-by: Alexander Beregalov <[email protected]>
> ---
>
> kernel/module.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 1721320..5a1258e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1835,8 +1835,10 @@ static struct module *load_module(void __user *umod,
> unsigned int markersindex;
> unsigned int markersstringsindex;
> unsigned int verboseindex;
> +#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
> struct mod_debug *iter;
> unsigned long value;
> +#endif
Hm, that's not very nice, we try to keep #ifdefs out of the .c file
where ever possible, especially within a single function. I'll split
the section out of this function that has the #ifdef which should fix
this problem.
thanks,
greg k-h
On Wed, Aug 20, 2008 at 07:24:53AM -0700, Greg KH wrote:
> On Wed, Aug 20, 2008 at 06:13:29PM +0400, Alexander Beregalov wrote:
> > From: Alexander Beregalov <[email protected]>
> >
> > kernel/module.c: In function 'load_module':
> > kernel/module.c:1839: warning: unused variable 'value'
> > kernel/module.c:1838: warning: unused variable 'iter'
> >
> >
> > Signed-off-by: Alexander Beregalov <[email protected]>
> > ---
> >
> > kernel/module.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 1721320..5a1258e 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -1835,8 +1835,10 @@ static struct module *load_module(void __user *umod,
> > unsigned int markersindex;
> > unsigned int markersstringsindex;
> > unsigned int verboseindex;
> > +#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
> > struct mod_debug *iter;
> > unsigned long value;
> > +#endif
>
> Hm, that's not very nice, we try to keep #ifdefs out of the .c file
> where ever possible, especially within a single function. I'll split
> the section out of this function that has the #ifdef which should fix
> this problem.
I'll merge in this patch below, unless someone objects.
thanks,
greg k-h
---
kernel/module.c | 54 ++++++++++++++++++++++++++++++++++--------------------
1 file changed, 34 insertions(+), 20 deletions(-)
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1778,12 +1778,44 @@ static void add_kallsyms(struct module *
static inline void add_kallsyms(struct module *mod,
Elf_Shdr *sechdrs,
unsigned int symindex,
- unsigned int strindex,
const char *secstrings)
{
}
#endif /* CONFIG_KALLSYMS */
+#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
+static void dynamic_printk_setup(struct module *mod,
+ Elf_Shdr *sechdrs,
+ unsigned int verboseindex,
+ const char *secstrings)
+{
+ struct mod_debug *iter;
+ unsigned long value;
+
+ mod->start_verbose = (void *)sechdrs[verboseindex].sh_addr;
+ mod->num_verbose = sechdrs[verboseindex].sh_size /
+ sizeof(*mod->start_verbose);
+
+ for (value = (unsigned long)mod->start_verbose;
+ value < (unsigned long)mod->start_verbose +
+ (unsigned long)(mod->num_verbose * sizeof(struct mod_debug));
+ value += sizeof(struct mod_debug)) {
+ iter = (struct mod_debug *)value;
+ register_dynamic_debug_module(iter->modname,
+ iter->type,
+ iter->logical_modname,
+ iter->flag_names, iter->hash, iter->hash2);
+ }
+}
+#else
+static inline void dynamic_printk_setup(struct module *mod,
+ Elf_Shdr *sechdrs,
+ unsigned int symindex,
+ const char *secstrings)
+{
+}
+#endif /* CONFIG_DYNAMIC_PRINTK_DEBUG */
+
static void *module_alloc_update_bounds(unsigned long size)
{
void *ret = module_alloc(size);
@@ -1833,8 +1865,6 @@ static struct module *load_module(void _
unsigned int markersindex;
unsigned int markersstringsindex;
unsigned int verboseindex;
- struct mod_debug *iter;
- unsigned long value;
struct module *mod;
long err = 0;
void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -2149,11 +2179,6 @@ static struct module *load_module(void _
mod->num_markers =
sechdrs[markersindex].sh_size / sizeof(*mod->markers);
#endif
-#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
- mod->start_verbose = (void *)sechdrs[verboseindex].sh_addr;
- mod->num_verbose = sechdrs[verboseindex].sh_size /
- sizeof(*mod->start_verbose);
-#endif
/* Find duplicate symbols */
err = verify_export_symbols(mod);
@@ -2177,18 +2202,7 @@ static struct module *load_module(void _
marker_update_probe_range(mod->markers,
mod->markers + mod->num_markers);
#endif
-#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
- for (value = (unsigned long)mod->start_verbose;
- value < (unsigned long)mod->start_verbose +
- (unsigned long)(mod->num_verbose * sizeof(struct mod_debug));
- value += sizeof(struct mod_debug)) {
- iter = (struct mod_debug *)value;
- register_dynamic_debug_module(iter->modname,
- iter->type,
- iter->logical_modname,
- iter->flag_names, iter->hash, iter->hash2);
- }
-#endif
+ dynamic_printk_setup(mod, sechdrs, verboseindex, secstrings);
err = module_finalize(hdr, sechdrs, mod);
if (err < 0)
goto cleanup;
On Wed, Aug 20, 2008 at 08:13:03AM -0700, Greg KH wrote:
> I'll merge in this patch below, unless someone objects.
>
> thanks,
>
> greg k-h
>
> ---
> kernel/module.c | 54 ++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 34 insertions(+), 20 deletions(-)
>
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1778,12 +1778,44 @@ static void add_kallsyms(struct module *
> static inline void add_kallsyms(struct module *mod,
> Elf_Shdr *sechdrs,
> unsigned int symindex,
> - unsigned int strindex,
> const char *secstrings)
> {
> }
> #endif /* CONFIG_KALLSYMS */
>
> +#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
> +static void dynamic_printk_setup(struct module *mod,
> + Elf_Shdr *sechdrs,
> + unsigned int verboseindex,
> + const char *secstrings)
> +{
> + struct mod_debug *iter;
> + unsigned long value;
> +
> + mod->start_verbose = (void *)sechdrs[verboseindex].sh_addr;
> + mod->num_verbose = sechdrs[verboseindex].sh_size /
> + sizeof(*mod->start_verbose);
> +
> + for (value = (unsigned long)mod->start_verbose;
> + value < (unsigned long)mod->start_verbose +
> + (unsigned long)(mod->num_verbose * sizeof(struct mod_debug));
> + value += sizeof(struct mod_debug)) {
> + iter = (struct mod_debug *)value;
> + register_dynamic_debug_module(iter->modname,
> + iter->type,
> + iter->logical_modname,
> + iter->flag_names, iter->hash, iter->hash2);
> + }
> +}
> +#else
> +static inline void dynamic_printk_setup(struct module *mod,
> + Elf_Shdr *sechdrs,
> + unsigned int symindex,
> + const char *secstrings)
> +{
> +}
> +#endif /* CONFIG_DYNAMIC_PRINTK_DEBUG */
> +
> static void *module_alloc_update_bounds(unsigned long size)
> {
> void *ret = module_alloc(size);
> @@ -1833,8 +1865,6 @@ static struct module *load_module(void _
> unsigned int markersindex;
> unsigned int markersstringsindex;
> unsigned int verboseindex;
> - struct mod_debug *iter;
> - unsigned long value;
> struct module *mod;
> long err = 0;
> void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
> @@ -2149,11 +2179,6 @@ static struct module *load_module(void _
> mod->num_markers =
> sechdrs[markersindex].sh_size / sizeof(*mod->markers);
> #endif
> -#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
> - mod->start_verbose = (void *)sechdrs[verboseindex].sh_addr;
> - mod->num_verbose = sechdrs[verboseindex].sh_size /
> - sizeof(*mod->start_verbose);
> -#endif
>
> /* Find duplicate symbols */
> err = verify_export_symbols(mod);
> @@ -2177,18 +2202,7 @@ static struct module *load_module(void _
> marker_update_probe_range(mod->markers,
> mod->markers + mod->num_markers);
> #endif
> -#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
> - for (value = (unsigned long)mod->start_verbose;
> - value < (unsigned long)mod->start_verbose +
> - (unsigned long)(mod->num_verbose * sizeof(struct mod_debug));
> - value += sizeof(struct mod_debug)) {
> - iter = (struct mod_debug *)value;
> - register_dynamic_debug_module(iter->modname,
> - iter->type,
> - iter->logical_modname,
> - iter->flag_names, iter->hash, iter->hash2);
> - }
> -#endif
> + dynamic_printk_setup(mod, sechdrs, verboseindex, secstrings);
> err = module_finalize(hdr, sechdrs, mod);
> if (err < 0)
> goto cleanup;
looks much cleaner. thanks.
we can simplify this module code a bit more...how about the following on top of
what you did?
thanks,
-Jason
Signed-off-by: Jason Baron <[email protected]>
---
include/linux/module.h | 5 -----
kernel/module.c | 40 +++++++++++++++++-----------------------
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 3336bd3..5d2970c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -350,11 +350,6 @@ struct module
/* Reference counts */
struct module_ref ref[NR_CPUS];
#endif
-
-#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
- struct mod_debug *start_verbose;
- unsigned int num_verbose;
-#endif
};
#ifndef MODULE_ARCH_INIT
#define MODULE_ARCH_INIT {}
diff --git a/kernel/module.c b/kernel/module.c
index b227ca4..dc5fa65 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1787,34 +1787,28 @@ static inline void add_kallsyms(struct module *mod,
#endif /* CONFIG_KALLSYMS */
#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
-static void dynamic_printk_setup(struct module *mod,
- Elf_Shdr *sechdrs,
- unsigned int verboseindex,
- const char *secstrings)
+static void dynamic_printk_setup(Elf_Shdr *sechdrs, unsigned int verboseindex)
{
- struct mod_debug *iter;
- unsigned long value;
+ struct mod_debug *debug_info;
+ unsigned long pos, end;
+ unsigned int num_verbose;
- mod->start_verbose = (void *)sechdrs[verboseindex].sh_addr;
- mod->num_verbose = sechdrs[verboseindex].sh_size /
- sizeof(*mod->start_verbose);
+ pos = sechdrs[verboseindex].sh_addr;
+ num_verbose = sechdrs[verboseindex].sh_size /
+ sizeof(struct mod_debug);
+ end = pos + (num_verbose * sizeof(struct mod_debug));
- for (value = (unsigned long)mod->start_verbose;
- value < (unsigned long)mod->start_verbose +
- (unsigned long)(mod->num_verbose * sizeof(struct mod_debug));
- value += sizeof(struct mod_debug)) {
- iter = (struct mod_debug *)value;
- register_dynamic_debug_module(iter->modname,
- iter->type,
- iter->logical_modname,
- iter->flag_names, iter->hash, iter->hash2);
+ for (; pos < end; pos += sizeof(struct mod_debug)) {
+ debug_info = (struct mod_debug *)pos;
+ register_dynamic_debug_module(debug_info->modname,
+ debug_info->type, debug_info->logical_modname,
+ debug_info->flag_names, debug_info->hash,
+ debug_info->hash2);
}
}
#else
-static inline void dynamic_printk_setup(struct module *mod,
- Elf_Shdr *sechdrs,
- unsigned int symindex,
- const char *secstrings)
+static inline void dynamic_printk_setup(Elf_Shdr *sechdrs,
+ unsigned int verboseindex)
{
}
#endif /* CONFIG_DYNAMIC_PRINTK_DEBUG */
@@ -2221,7 +2215,7 @@ static struct module *load_module(void __user *umod,
mod->tracepoints + mod->num_tracepoints);
#endif
}
- dynamic_printk_setup(mod, sechdrs, verboseindex, secstrings);
+ dynamic_printk_setup(sechdrs, verboseindex);
err = module_finalize(hdr, sechdrs, mod);
if (err < 0)
goto cleanup;
On Thu, Aug 21, 2008 at 04:35:39PM -0400, Jason Baron wrote:
> looks much cleaner. thanks.
>
> we can simplify this module code a bit more...how about the following on top of
> what you did?
>
> thanks,
>
> -Jason
>
or better yet, without trailing whitespace...
Signed-off-by: Jason Baron <[email protected]>
---
include/linux/module.h | 5 -----
kernel/module.c | 40 +++++++++++++++++-----------------------
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 3336bd3..5d2970c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -350,11 +350,6 @@ struct module
/* Reference counts */
struct module_ref ref[NR_CPUS];
#endif
-
-#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
- struct mod_debug *start_verbose;
- unsigned int num_verbose;
-#endif
};
#ifndef MODULE_ARCH_INIT
#define MODULE_ARCH_INIT {}
diff --git a/kernel/module.c b/kernel/module.c
index b227ca4..2f69b81 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1787,34 +1787,28 @@ static inline void add_kallsyms(struct module *mod,
#endif /* CONFIG_KALLSYMS */
#ifdef CONFIG_DYNAMIC_PRINTK_DEBUG
-static void dynamic_printk_setup(struct module *mod,
- Elf_Shdr *sechdrs,
- unsigned int verboseindex,
- const char *secstrings)
+static void dynamic_printk_setup(Elf_Shdr *sechdrs, unsigned int verboseindex)
{
- struct mod_debug *iter;
- unsigned long value;
+ struct mod_debug *debug_info;
+ unsigned long pos, end;
+ unsigned int num_verbose;
- mod->start_verbose = (void *)sechdrs[verboseindex].sh_addr;
- mod->num_verbose = sechdrs[verboseindex].sh_size /
- sizeof(*mod->start_verbose);
+ pos = sechdrs[verboseindex].sh_addr;
+ num_verbose = sechdrs[verboseindex].sh_size /
+ sizeof(struct mod_debug);
+ end = pos + (num_verbose * sizeof(struct mod_debug));
- for (value = (unsigned long)mod->start_verbose;
- value < (unsigned long)mod->start_verbose +
- (unsigned long)(mod->num_verbose * sizeof(struct mod_debug));
- value += sizeof(struct mod_debug)) {
- iter = (struct mod_debug *)value;
- register_dynamic_debug_module(iter->modname,
- iter->type,
- iter->logical_modname,
- iter->flag_names, iter->hash, iter->hash2);
+ for (; pos < end; pos += sizeof(struct mod_debug)) {
+ debug_info = (struct mod_debug *)pos;
+ register_dynamic_debug_module(debug_info->modname,
+ debug_info->type, debug_info->logical_modname,
+ debug_info->flag_names, debug_info->hash,
+ debug_info->hash2);
}
}
#else
-static inline void dynamic_printk_setup(struct module *mod,
- Elf_Shdr *sechdrs,
- unsigned int symindex,
- const char *secstrings)
+static inline void dynamic_printk_setup(Elf_Shdr *sechdrs,
+ unsigned int verboseindex)
{
}
#endif /* CONFIG_DYNAMIC_PRINTK_DEBUG */
@@ -2221,7 +2215,7 @@ static struct module *load_module(void __user *umod,
mod->tracepoints + mod->num_tracepoints);
#endif
}
- dynamic_printk_setup(mod, sechdrs, verboseindex, secstrings);
+ dynamic_printk_setup(sechdrs, verboseindex);
err = module_finalize(hdr, sechdrs, mod);
if (err < 0)
goto cleanup;