* [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG @ 2008-08-20 14:13 Alexander Beregalov 2008-08-20 14:24 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Alexander Beregalov @ 2008-08-20 14:13 UTC (permalink / raw) To: jbaron, gregkh, linux-kernel From: Alexander Beregalov <a.beregalov@gmail.com> 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 <a.beregalov@gmail.com> --- 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; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG 2008-08-20 14:13 [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG Alexander Beregalov @ 2008-08-20 14:24 ` Greg KH 2008-08-20 15:13 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2008-08-20 14:24 UTC (permalink / raw) To: Alexander Beregalov; +Cc: jbaron, linux-kernel On Wed, Aug 20, 2008 at 06:13:29PM +0400, Alexander Beregalov wrote: > From: Alexander Beregalov <a.beregalov@gmail.com> > > 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 <a.beregalov@gmail.com> > --- > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG 2008-08-20 14:24 ` Greg KH @ 2008-08-20 15:13 ` Greg KH 2008-08-21 20:35 ` Jason Baron 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2008-08-20 15:13 UTC (permalink / raw) To: Alexander Beregalov; +Cc: jbaron, linux-kernel 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 <a.beregalov@gmail.com> > > > > 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 <a.beregalov@gmail.com> > > --- > > > > 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG 2008-08-20 15:13 ` Greg KH @ 2008-08-21 20:35 ` Jason Baron 2008-08-21 20:45 ` Jason Baron 0 siblings, 1 reply; 6+ messages in thread From: Jason Baron @ 2008-08-21 20:35 UTC (permalink / raw) To: Greg KH; +Cc: Alexander Beregalov, linux-kernel 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 <jbaron@redhat.com> --- 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; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG 2008-08-21 20:35 ` Jason Baron @ 2008-08-21 20:45 ` Jason Baron 2008-09-11 9:31 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Jason Baron @ 2008-08-21 20:45 UTC (permalink / raw) To: Greg KH; +Cc: Alexander Beregalov, linux-kernel 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 <jbaron@redhat.com> --- 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; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG 2008-08-21 20:45 ` Jason Baron @ 2008-09-11 9:31 ` Greg KH 0 siblings, 0 replies; 6+ messages in thread From: Greg KH @ 2008-09-11 9:31 UTC (permalink / raw) To: Jason Baron; +Cc: Greg KH, Alexander Beregalov, linux-kernel On Thu, Aug 21, 2008 at 04:45:13PM -0400, Jason Baron wrote: > 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... Looks good, thanks, I've merged it in. greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-09-11 10:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-08-20 14:13 [PATCH] kernel/module: fix warning when !CONFIG_DYNAMIC_PRINTK_DEBUG Alexander Beregalov 2008-08-20 14:24 ` Greg KH 2008-08-20 15:13 ` Greg KH 2008-08-21 20:35 ` Jason Baron 2008-08-21 20:45 ` Jason Baron 2008-09-11 9:31 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox