Re: [PATCH] kbuild: implement modules.order, take #2
On Fri, Dec 07, 2007 at 09:04:30PM +0900, Tejun Heo wrote: > When multiple built-in modules (especially drivers) provide the same > capability, they're prioritized by link order specified by the order > listed in Makefile. This implicit ordering is lost for loadable > modules. > > When driver modules are loaded by udev, what comes first in > modules.alias file is selected. However, the order in this file is > indeterministic (depends on filesystem listing order of installed > modules). This causes confusion. > > The solution is two-parted. This patch updates kbuild such that it > generates and installs modules.order which contains the name of > modules ordered according to Makefile. The second part is update to > depmod such that it generates output files according to this file. > > Note that both obj-y and obj-m subdirs can contain modules and > ordering information between those two are lost from beginning. > Currently obj-y subdirs are put before obj-m subdirs. > > Sam Ravnborg cleaned up Makefile modifications and suggested using awk > to remove duplicate lines from modules.order instead of using separate > C program. > > Signed-off-by: Tejun Heo <[EMAIL PROTECTED]> > Cc: Sam Ravnborg <[EMAIL PROTECTED]> > Cc: Bill Nottingham <[EMAIL PROTECTED]> > Cc: Rusty Russell <[EMAIL PROTECTED]> > Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Acked-by: Greg Kroah-Hartman <[EMAIL PROTECTED]> thanks for doing this, looks very nice. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order, take #2
On Fri, Dec 07, 2007 at 09:04:30PM +0900, Tejun Heo wrote: When multiple built-in modules (especially drivers) provide the same capability, they're prioritized by link order specified by the order listed in Makefile. This implicit ordering is lost for loadable modules. When driver modules are loaded by udev, what comes first in modules.alias file is selected. However, the order in this file is indeterministic (depends on filesystem listing order of installed modules). This causes confusion. The solution is two-parted. This patch updates kbuild such that it generates and installs modules.order which contains the name of modules ordered according to Makefile. The second part is update to depmod such that it generates output files according to this file. Note that both obj-y and obj-m subdirs can contain modules and ordering information between those two are lost from beginning. Currently obj-y subdirs are put before obj-m subdirs. Sam Ravnborg cleaned up Makefile modifications and suggested using awk to remove duplicate lines from modules.order instead of using separate C program. Signed-off-by: Tejun Heo [EMAIL PROTECTED] Cc: Sam Ravnborg [EMAIL PROTECTED] Cc: Bill Nottingham [EMAIL PROTECTED] Cc: Rusty Russell [EMAIL PROTECTED] Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Acked-by: Greg Kroah-Hartman [EMAIL PROTECTED] thanks for doing this, looks very nice. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order, take #2
On Fri, Dec 07, 2007 at 09:04:30PM +0900, Tejun Heo wrote: > When multiple built-in modules (especially drivers) provide the same > capability, they're prioritized by link order specified by the order > listed in Makefile. This implicit ordering is lost for loadable > modules. > > When driver modules are loaded by udev, what comes first in > modules.alias file is selected. However, the order in this file is > indeterministic (depends on filesystem listing order of installed > modules). This causes confusion. > > The solution is two-parted. This patch updates kbuild such that it > generates and installs modules.order which contains the name of > modules ordered according to Makefile. The second part is update to > depmod such that it generates output files according to this file. > > Note that both obj-y and obj-m subdirs can contain modules and > ordering information between those two are lost from beginning. > Currently obj-y subdirs are put before obj-m subdirs. > > Sam Ravnborg cleaned up Makefile modifications and suggested using awk > to remove duplicate lines from modules.order instead of using separate > C program. > > Signed-off-by: Tejun Heo <[EMAIL PROTECTED]> > Cc: Sam Ravnborg <[EMAIL PROTECTED]> > Cc: Bill Nottingham <[EMAIL PROTECTED]> > Cc: Rusty Russell <[EMAIL PROTECTED]> > Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> > Cc: Kay Sievers <[EMAIL PROTECTED]> Applied to kbuild.git. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order, take #2
On Fri, Dec 07, 2007 at 09:04:30PM +0900, Tejun Heo wrote: When multiple built-in modules (especially drivers) provide the same capability, they're prioritized by link order specified by the order listed in Makefile. This implicit ordering is lost for loadable modules. When driver modules are loaded by udev, what comes first in modules.alias file is selected. However, the order in this file is indeterministic (depends on filesystem listing order of installed modules). This causes confusion. The solution is two-parted. This patch updates kbuild such that it generates and installs modules.order which contains the name of modules ordered according to Makefile. The second part is update to depmod such that it generates output files according to this file. Note that both obj-y and obj-m subdirs can contain modules and ordering information between those two are lost from beginning. Currently obj-y subdirs are put before obj-m subdirs. Sam Ravnborg cleaned up Makefile modifications and suggested using awk to remove duplicate lines from modules.order instead of using separate C program. Signed-off-by: Tejun Heo [EMAIL PROTECTED] Cc: Sam Ravnborg [EMAIL PROTECTED] Cc: Bill Nottingham [EMAIL PROTECTED] Cc: Rusty Russell [EMAIL PROTECTED] Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Cc: Kay Sievers [EMAIL PROTECTED] Applied to kbuild.git. Sam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
Adrian Bunk wrote: > And thinking about it, it doesn't seem to add any problems regarding > what I have in mind: > > If we would ever stop having any well-defined link-order for in-kernel > code and express everything through initcall levels, we simply must > additionally update the modules.order file. Expressing order in Makefile is a convenient way to express simple ordering. I think it's nice to keep it that way even if it doesn't necessarily mean link order. So, maybe we can do the reverse and make built in modules follow module order regardless of actual link order (say sort init table according to module order before final link). Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
On Sat, Dec 08, 2007 at 08:59:31AM +0900, Tejun Heo wrote: > Adrian Bunk wrote: > > On Tue, Dec 04, 2007 at 10:49:37PM +0900, Tejun Heo wrote: > >> When multiple built-in modules (especially drivers) provide the same > >> capability, they're prioritized by link order specified by the order > >> listed in Makefile. This implicit ordering is lost for loadable > >> modules. > >> ... > > > > What exactly are the drivers you are thinking of? > > > > I would rather see us getting away from any link order dependencies. > > > > E.g. we might one day want to compile the whole kernel with one gcc call > > (using "--combine -fwhole-program"). > > The following bugzilla triggered this change and I think contains enough > discussion on the subject. > > http://bugzilla.kernel.org/show_bug.cgi?id=8933 > > Thanks. Thanks, that explains much. And thinking about it, it doesn't seem to add any problems regarding what I have in mind: If we would ever stop having any well-defined link-order for in-kernel code and express everything through initcall levels, we simply must additionally update the modules.order file. > tejun cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
On Sat, Dec 08, 2007 at 08:59:31AM +0900, Tejun Heo wrote: Adrian Bunk wrote: On Tue, Dec 04, 2007 at 10:49:37PM +0900, Tejun Heo wrote: When multiple built-in modules (especially drivers) provide the same capability, they're prioritized by link order specified by the order listed in Makefile. This implicit ordering is lost for loadable modules. ... What exactly are the drivers you are thinking of? I would rather see us getting away from any link order dependencies. E.g. we might one day want to compile the whole kernel with one gcc call (using --combine -fwhole-program). The following bugzilla triggered this change and I think contains enough discussion on the subject. http://bugzilla.kernel.org/show_bug.cgi?id=8933 Thanks. Thanks, that explains much. And thinking about it, it doesn't seem to add any problems regarding what I have in mind: If we would ever stop having any well-defined link-order for in-kernel code and express everything through initcall levels, we simply must additionally update the modules.order file. tejun cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
Adrian Bunk wrote: And thinking about it, it doesn't seem to add any problems regarding what I have in mind: If we would ever stop having any well-defined link-order for in-kernel code and express everything through initcall levels, we simply must additionally update the modules.order file. Expressing order in Makefile is a convenient way to express simple ordering. I think it's nice to keep it that way even if it doesn't necessarily mean link order. So, maybe we can do the reverse and make built in modules follow module order regardless of actual link order (say sort init table according to module order before final link). Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
Adrian Bunk wrote: > On Tue, Dec 04, 2007 at 10:49:37PM +0900, Tejun Heo wrote: >> When multiple built-in modules (especially drivers) provide the same >> capability, they're prioritized by link order specified by the order >> listed in Makefile. This implicit ordering is lost for loadable >> modules. >> ... > > What exactly are the drivers you are thinking of? > > I would rather see us getting away from any link order dependencies. > > E.g. we might one day want to compile the whole kernel with one gcc call > (using "--combine -fwhole-program"). The following bugzilla triggered this change and I think contains enough discussion on the subject. http://bugzilla.kernel.org/show_bug.cgi?id=8933 Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
On Tue, Dec 04, 2007 at 10:49:37PM +0900, Tejun Heo wrote: > When multiple built-in modules (especially drivers) provide the same > capability, they're prioritized by link order specified by the order > listed in Makefile. This implicit ordering is lost for loadable > modules. >... What exactly are the drivers you are thinking of? I would rather see us getting away from any link order dependencies. E.g. we might one day want to compile the whole kernel with one gcc call (using "--combine -fwhole-program"). cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kbuild: implement modules.order, take #2
When multiple built-in modules (especially drivers) provide the same capability, they're prioritized by link order specified by the order listed in Makefile. This implicit ordering is lost for loadable modules. When driver modules are loaded by udev, what comes first in modules.alias file is selected. However, the order in this file is indeterministic (depends on filesystem listing order of installed modules). This causes confusion. The solution is two-parted. This patch updates kbuild such that it generates and installs modules.order which contains the name of modules ordered according to Makefile. The second part is update to depmod such that it generates output files according to this file. Note that both obj-y and obj-m subdirs can contain modules and ordering information between those two are lost from beginning. Currently obj-y subdirs are put before obj-m subdirs. Sam Ravnborg cleaned up Makefile modifications and suggested using awk to remove duplicate lines from modules.order instead of using separate C program. Signed-off-by: Tejun Heo <[EMAIL PROTECTED]> Cc: Sam Ravnborg <[EMAIL PROTECTED]> Cc: Bill Nottingham <[EMAIL PROTECTED]> Cc: Rusty Russell <[EMAIL PROTECTED]> Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]> Cc: Kay Sievers <[EMAIL PROTECTED]> --- Makefile |8 +++- scripts/Makefile.build | 17 - scripts/Makefile.lib |6 ++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 92dc3cb..1542dd2 100644 --- a/Makefile +++ b/Makefile @@ -1020,9 +1020,14 @@ ifdef CONFIG_MODULES all: modules # Build modules +# +# A module can be listed more than once in obj-m resulting in +# duplicate lines in modules.order files. Those are removed +# using awk while concatenating to the final file. PHONY += modules modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) + $(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order @echo ' Building modules, stage 2.'; $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost @@ -1050,6 +1055,7 @@ _modinst_: rm -f $(MODLIB)/build ; \ ln -s $(objtree) $(MODLIB)/build ; \ fi + @cp -f $(objtree)/modules.order $(MODLIB)/ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst # This depmod is only for convenience to give the initial @@ -1109,7 +1115,7 @@ clean: archclean $(clean-dirs) @find . $(RCS_FIND_IGNORE) \ \( -name '*.[oas]' -o -name '*.ko' -o -name '.*.cmd' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ - -o -name '*.symtypes' \) \ + -o -name '*.symtypes' -o -name 'modules.order' \) \ -type f -print | xargs rm -f # mrproper - Delete all generated files, including .config diff --git a/scripts/Makefile.build b/scripts/Makefile.build index de9836e..875cbdb 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -83,10 +83,12 @@ ifneq ($(strip $(obj-y) $(obj-m) $(obj-n) $(obj-) $(lib-target)),) builtin-target := $(obj)/built-in.o endif +modorder-target := $(obj)/modules.order + # We keep a list of all modules in $(MODVERDIR) __build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \ -$(if $(KBUILD_MODULES),$(obj-m)) \ +$(if $(KBUILD_MODULES),$(obj-m) $(modorder-target)) \ $(subdir-ym) $(always) @: @@ -276,6 +278,19 @@ targets += $(builtin-target) endif # builtin-target # +# Rule to create modules.order file +# +# Create commands to either record .ko file or cat modules.order from +# a subdirectory +modorder-cmds =\ + $(foreach m, $(modorder), \ + $(if $(filter %/modules.order, $m), \ + cat $m;, echo kernel/$m;)) + +$(modorder-target): $(subdir-ym) FORCE + $(Q)(cat /dev/null; $(modorder-cmds)) > $@ + +# # Rule to compile a set of .o files into one .a file # ifdef lib-target diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 3c5e88b..8e44023 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -25,6 +25,11 @@ lib-y := $(filter-out $(obj-y), $(sort $(lib-y) $(lib-m))) # o if we encounter foo/ in $(obj-m), remove it from $(obj-m) # and add the directory to the list of dirs to descend into: $(subdir-m) +# Determine modorder. +# Unfortunately, we don't have information about ordering between -y +# and -m subdirs. Just put -y's first. +modorder := $(patsubst %/,%/modules.order, $(filter %/, $(obj-y)) $(obj-m:.o=.ko)) + __subdir-y := $(patsubst %/,%,$(filter %/, $(obj-y))) subdir-y += $(__subdir-y) __subdir-m := $(patsubst %/,%,$(filter %/, $(obj-m))) @@ -64,6 +69,7 @@ real-objs-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y) extra-y:= $(addprefix
[PATCH] kbuild: implement modules.order, take #2
When multiple built-in modules (especially drivers) provide the same capability, they're prioritized by link order specified by the order listed in Makefile. This implicit ordering is lost for loadable modules. When driver modules are loaded by udev, what comes first in modules.alias file is selected. However, the order in this file is indeterministic (depends on filesystem listing order of installed modules). This causes confusion. The solution is two-parted. This patch updates kbuild such that it generates and installs modules.order which contains the name of modules ordered according to Makefile. The second part is update to depmod such that it generates output files according to this file. Note that both obj-y and obj-m subdirs can contain modules and ordering information between those two are lost from beginning. Currently obj-y subdirs are put before obj-m subdirs. Sam Ravnborg cleaned up Makefile modifications and suggested using awk to remove duplicate lines from modules.order instead of using separate C program. Signed-off-by: Tejun Heo [EMAIL PROTECTED] Cc: Sam Ravnborg [EMAIL PROTECTED] Cc: Bill Nottingham [EMAIL PROTECTED] Cc: Rusty Russell [EMAIL PROTECTED] Cc: Greg Kroah-Hartman [EMAIL PROTECTED] Cc: Kay Sievers [EMAIL PROTECTED] --- Makefile |8 +++- scripts/Makefile.build | 17 - scripts/Makefile.lib |6 ++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 92dc3cb..1542dd2 100644 --- a/Makefile +++ b/Makefile @@ -1020,9 +1020,14 @@ ifdef CONFIG_MODULES all: modules # Build modules +# +# A module can be listed more than once in obj-m resulting in +# duplicate lines in modules.order files. Those are removed +# using awk while concatenating to the final file. PHONY += modules modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) + $(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) $(objtree)/modules.order @echo ' Building modules, stage 2.'; $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost @@ -1050,6 +1055,7 @@ _modinst_: rm -f $(MODLIB)/build ; \ ln -s $(objtree) $(MODLIB)/build ; \ fi + @cp -f $(objtree)/modules.order $(MODLIB)/ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst # This depmod is only for convenience to give the initial @@ -1109,7 +1115,7 @@ clean: archclean $(clean-dirs) @find . $(RCS_FIND_IGNORE) \ \( -name '*.[oas]' -o -name '*.ko' -o -name '.*.cmd' \ -o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \ - -o -name '*.symtypes' \) \ + -o -name '*.symtypes' -o -name 'modules.order' \) \ -type f -print | xargs rm -f # mrproper - Delete all generated files, including .config diff --git a/scripts/Makefile.build b/scripts/Makefile.build index de9836e..875cbdb 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -83,10 +83,12 @@ ifneq ($(strip $(obj-y) $(obj-m) $(obj-n) $(obj-) $(lib-target)),) builtin-target := $(obj)/built-in.o endif +modorder-target := $(obj)/modules.order + # We keep a list of all modules in $(MODVERDIR) __build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \ -$(if $(KBUILD_MODULES),$(obj-m)) \ +$(if $(KBUILD_MODULES),$(obj-m) $(modorder-target)) \ $(subdir-ym) $(always) @: @@ -276,6 +278,19 @@ targets += $(builtin-target) endif # builtin-target # +# Rule to create modules.order file +# +# Create commands to either record .ko file or cat modules.order from +# a subdirectory +modorder-cmds =\ + $(foreach m, $(modorder), \ + $(if $(filter %/modules.order, $m), \ + cat $m;, echo kernel/$m;)) + +$(modorder-target): $(subdir-ym) FORCE + $(Q)(cat /dev/null; $(modorder-cmds)) $@ + +# # Rule to compile a set of .o files into one .a file # ifdef lib-target diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 3c5e88b..8e44023 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -25,6 +25,11 @@ lib-y := $(filter-out $(obj-y), $(sort $(lib-y) $(lib-m))) # o if we encounter foo/ in $(obj-m), remove it from $(obj-m) # and add the directory to the list of dirs to descend into: $(subdir-m) +# Determine modorder. +# Unfortunately, we don't have information about ordering between -y +# and -m subdirs. Just put -y's first. +modorder := $(patsubst %/,%/modules.order, $(filter %/, $(obj-y)) $(obj-m:.o=.ko)) + __subdir-y := $(patsubst %/,%,$(filter %/, $(obj-y))) subdir-y += $(__subdir-y) __subdir-m := $(patsubst %/,%,$(filter %/, $(obj-m))) @@ -64,6 +69,7 @@ real-objs-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y) extra-y:= $(addprefix $(obj)/,$(extra-y))
Re: [PATCH] kbuild: implement modules.order
Adrian Bunk wrote: On Tue, Dec 04, 2007 at 10:49:37PM +0900, Tejun Heo wrote: When multiple built-in modules (especially drivers) provide the same capability, they're prioritized by link order specified by the order listed in Makefile. This implicit ordering is lost for loadable modules. ... What exactly are the drivers you are thinking of? I would rather see us getting away from any link order dependencies. E.g. we might one day want to compile the whole kernel with one gcc call (using --combine -fwhole-program). The following bugzilla triggered this change and I think contains enough discussion on the subject. http://bugzilla.kernel.org/show_bug.cgi?id=8933 Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
On Tue, Dec 04, 2007 at 10:49:37PM +0900, Tejun Heo wrote: When multiple built-in modules (especially drivers) provide the same capability, they're prioritized by link order specified by the order listed in Makefile. This implicit ordering is lost for loadable modules. ... What exactly are the drivers you are thinking of? I would rather see us getting away from any link order dependencies. E.g. we might one day want to compile the whole kernel with one gcc call (using --combine -fwhole-program). cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
On Wednesday 05 December 2007 18:11:49 Tejun Heo wrote: > WANG Cong wrote: > >>> I think, you forgot to free(3) the memory you calloc(3)'ed and > >>> malloc(3)'ed above. > >> > >> It's a simple program where whole body is in main(). Why bother? > >> What's the benefit of adding hash-table iterating free logic? > > > > Personally, I think memory leaks are bugs. And we hate bugs. ;) > > Trust me. As a person buried alive in bug reports, I hate bugs too. I > just don't agree that this type of programs should free all its > resources before exiting. How about adding a comment saying /* we're > going out anyway, don't bother freeing hashtable */? I too once battled with the moral dilemma of freeing in programs that exit. Then in 2001, I was moving out of a house which was to be demolished. The landlord insisted that we pay for the carpets to be cleaned. My wife still uses it as a canonical example of wasteful idiocy. So I hope this has contributed to your enlightenment, as it did to mine. Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
On Wednesday 05 December 2007 18:11:49 Tejun Heo wrote: WANG Cong wrote: I think, you forgot to free(3) the memory you calloc(3)'ed and malloc(3)'ed above. It's a simple program where whole body is in main(). Why bother? What's the benefit of adding hash-table iterating free logic? Personally, I think memory leaks are bugs. And we hate bugs. ;) Trust me. As a person buried alive in bug reports, I hate bugs too. I just don't agree that this type of programs should free all its resources before exiting. How about adding a comment saying /* we're going out anyway, don't bother freeing hashtable */? I too once battled with the moral dilemma of freeing in programs that exit. Then in 2001, I was moving out of a house which was to be demolished. The landlord insisted that we pay for the carpets to be cleaned. My wife still uses it as a canonical example of wasteful idiocy. So I hope this has contributed to your enlightenment, as it did to mine. Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
Tejun Heo wrote: > WANG Cong wrote: I think, you forgot to free(3) the memory you calloc(3)'ed and malloc(3)'ed above. >>> It's a simple program where whole body is in main(). Why bother? >>> What's the benefit of adding hash-table iterating free logic? >>> >> Personally, I think memory leaks are bugs. And we hate bugs. ;) > > Trust me. As a person buried alive in bug reports, I hate bugs too. I > just don't agree that this type of programs should free all its > resources before exiting. How about adding a comment saying /* we're > going out anyway, don't bother freeing hashtable */? > I don't consider this as memory leak. I agree that a comment should be enough. :) Li Zefam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
WANG Cong wrote: >>> I think, you forgot to free(3) the memory you calloc(3)'ed and >>> malloc(3)'ed above. >> It's a simple program where whole body is in main(). Why bother? >> What's the benefit of adding hash-table iterating free logic? >> > > Personally, I think memory leaks are bugs. And we hate bugs. ;) Trust me. As a person buried alive in bug reports, I hate bugs too. I just don't agree that this type of programs should free all its resources before exiting. How about adding a comment saying /* we're going out anyway, don't bother freeing hashtable */? -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
>> I think, you forgot to free(3) the memory you calloc(3)'ed and >> malloc(3)'ed above. > >It's a simple program where whole body is in main(). Why bother? >What's the benefit of adding hash-table iterating free logic? > Personally, I think memory leaks are bugs. And we hate bugs. ;) Regards. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
WANG Cong wrote:
>> +static inline unsigned int sdb_hash(const char *str)
>> +{
>> +unsigned int hash = 0;
>> +int c;
>> +
>> +while ((c = *str++))
>
> Maybe ` while ((c = *str++) != '\0') ` is better. ;)
Yeah, probably. That hash function is copied & pasted mindlessly from web.
>> +hash = c + (hash << 6) + (hash << 16) - hash;
>> +
>> +return hash;
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +unsigned int nr_entries = 0;
>> +struct hash_ent **hash_tbl;
>> +char line[10240];
>
> Needs to #define the magic number?
Or maybe write a wrapper function around fgets() to detect long lines
reliably.
>> +while (fgets(line, sizeof(line), fp)) {
>> +int len = strlen(line);
>
> strlen returns 'size_t', which is unsigned.
It's capped by the magic number above but yeah size_t would be better.
> I think, you forgot to free(3) the memory you calloc(3)'ed and
> malloc(3)'ed above.
It's a simple program where whole body is in main(). Why bother?
What's the benefit of adding hash-table iterating free logic?
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
{snip}
Comments on your C code below.
>--- /dev/null
>+++ b/scripts/remove-dup.c
>@@ -0,0 +1,98 @@
>+/*
>+ * remove-dup - Drop duplicate lines from unsorted input files
>+ *
>+ * Dec 2007 Tejun Heo <[EMAIL PROTECTED]>
>+ *
>+ * This software is released under GPLv2.
>+ */
>+
>+#include
>+#include
>+#include
>+
>+struct hash_ent {
>+ struct hash_ent *next;
>+ char str[];
>+};
>+
>+#define fatal(fmt, args...) do {\
>+ fprintf(stderr, fmt , ##args); \
>+ exit(1);\
>+ } while (0)
>+
>+static inline unsigned int sdb_hash(const char *str)
>+{
>+unsigned int hash = 0;
>+int c;
>+
>+while ((c = *str++))
Maybe ` while ((c = *str++) != '\0') ` is better. ;)
>+ hash = c + (hash << 6) + (hash << 16) - hash;
>+
>+return hash;
>+}
>+
>+int main(int argc, char **argv)
>+{
>+ unsigned int nr_entries = 0;
>+ struct hash_ent **hash_tbl;
>+ char line[10240];
Needs to #define the magic number?
>+ int i;
>+
>+ /* first pass, count lines */
>+ for (i = 1; i < argc; i++) {
>+ FILE *fp = fopen(argv[i], "r");
>+
>+ if (!fp)
>+ fatal("failed to open %s for reading\n", argv[i]);
>+
>+ while (fgets(line, sizeof(line), fp))
>+ nr_entries++;
>+
>+ fclose(fp);
>+ }
>+
>+ nr_entries = nr_entries * 10 / 8;
>+ hash_tbl = calloc(nr_entries, sizeof(struct hash_ent *));
>+ if (!hash_tbl)
>+ fatal("failed to allocate hash table for %u entries\n",
>+nr_entries);
>+
>+ /* second pass, hash and print unique lines */
>+ for (i = 1; i < argc; i++) {
>+ FILE *fp = fopen(argv[i], "r");
>+
>+ if (!fp)
>+ fatal("failed to open %s for reading\n", argv[i]);
>+
>+ while (fgets(line, sizeof(line), fp)) {
>+ int len = strlen(line);
strlen returns 'size_t', which is unsigned.
>+ struct hash_ent **ppos, *new_ent;
>+
>+ if (line[len - 1] == '\n')
>+ line[--len] = '\0';
>+
>+ ppos = hash_tbl + (sdb_hash(line) % nr_entries);
>+ while (*ppos) {
>+ if (strcmp((*ppos)->str, line) == 0)
>+ break;
>+ ppos = &(*ppos)->next;
>+ }
>+ if (*ppos)
>+ continue;
>+
>+ new_ent = malloc(sizeof(struct hash_ent) + len + 1);
>+ if (!new_ent)
>+ fatal("failed to allocate hash entry\n");
>+ new_ent->next = NULL;
>+ memcpy(new_ent->str, line, len + 1);
>+
>+ *ppos = new_ent;
>+
>+ printf("%s\n", line);
>+ }
>+
>+ fclose(fp);
>+ }
>+
I think, you forgot to free(3) the memory you calloc(3)'ed and
malloc(3)'ed above.
>+ return 0;
>+}
Thanks!
Cong
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] kbuild: implement modules.order
When multiple built-in modules (especially drivers) provide the same
capability, they're prioritized by link order specified by the order
listed in Makefile. This implicit ordering is lost for loadable
modules.
When driver modules are loaded by udev, what comes first in
modules.alias file is selected. However, the order in this file is
indeterministic (depends on filesystem listing order of installed
modules). This causes confusion.
The solution is two-parted. This patch updates kbuild such that it
generates and installs modules.order which contains the name of
modules ordered according to Makefile. The second part is update to
depmod such that it generates output files according to this file.
Note that both obj-y and obj-m subdirs can contain modules and
ordering information between those two are lost from beginning.
Currently obj-y subdirs are put before obj-m subdirs.
Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: Bill Nottingham <[EMAIL PROTECTED]>
Cc: Rusty Russell <[EMAIL PROTECTED]>
Cc: Greg Kroah-Hartman <[EMAIL PROTECTED]>
Cc: Kay Sievers <[EMAIL PROTECTED]>
---
Makefile |5 ++
scripts/Makefile |1
scripts/Makefile.build | 16 +++-
scripts/Makefile.lib | 10 +
scripts/remove-dup.c | 98 +
5 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index a65ffd2..1c020c7 100644
--- a/Makefile
+++ b/Makefile
@@ -311,6 +311,7 @@ DEPMOD = /sbin/depmod
KALLSYMS = scripts/kallsyms
PERL = perl
CHECK = sparse
+REMOVE_DUP = scripts/remove-dup
CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise
$(CF)
MODFLAGS = -DMODULE
@@ -1023,6 +1024,7 @@ all: modules
PHONY += modules
modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux)
+ @$(REMOVE_DUP) $(vmlinux-dirs:%=$(objtree)/%/modules.order) >
$(objtree)/modules.order
@echo ' Building modules, stage 2.';
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
@@ -1050,6 +1052,7 @@ _modinst_:
rm -f $(MODLIB)/build ; \
ln -s $(objtree) $(MODLIB)/build ; \
fi
+ @cp -f $(objtree)/modules.order $(MODLIB)/
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
# This depmod is only for convenience to give the initial
@@ -1109,7 +1112,7 @@ clean: archclean $(clean-dirs)
@find . $(RCS_FIND_IGNORE) \
\( -name '*.[oas]' -o -name '*.ko' -o -name '.*.cmd' \
-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
- -o -name '*.symtypes' \) \
+ -o -name '*.symtypes' -o -name 'modules.order' \) \
-type f -print | xargs rm -f
# mrproper - Delete all generated files, including .config
diff --git a/scripts/Makefile b/scripts/Makefile
index 1c73c5a..1c82547 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -12,6 +12,7 @@ hostprogs-$(CONFIG_LOGO) += pnmtologo
hostprogs-$(CONFIG_VT) += conmakehash
hostprogs-$(CONFIG_PROM_CONSOLE) += conmakehash
hostprogs-$(CONFIG_IKCONFIG) += bin2c
+hostprogs-$(CONFIG_MODULES) += remove-dup
always := $(hostprogs-y) $(hostprogs-m)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index de9836e..816a955 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -83,10 +83,12 @@ ifneq ($(strip $(obj-y) $(obj-m) $(obj-n) $(obj-)
$(lib-target)),)
builtin-target := $(obj)/built-in.o
endif
+modorder-target := $(obj)/modules.order
+
# We keep a list of all modules in $(MODVERDIR)
__build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
-$(if $(KBUILD_MODULES),$(obj-m)) \
+$(if $(KBUILD_MODULES),$(obj-m) $(modorder-target)) \
$(subdir-ym) $(always)
@:
@@ -276,6 +278,18 @@ targets += $(builtin-target)
endif # builtin-target
#
+# Rule to create modules.order file
+#
+$(modorder-target): $(subdir-ym) FORCE
+ @{ for m in $(modorder); do \
+ if echo $$m | grep -q '.*/modules.order'; then \
+ cat $$m;\
+ else\
+ echo kernel/$$m;\
+ fi; \
+ done } > $@
+
+#
# Rule to compile a set of .o files into one .a file
#
ifdef lib-target
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 3c5e88b..4563c96 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -25,6 +25,15 @@ lib-y := $(filter-out $(obj-y), $(sort $(lib-y) $(lib-m)))
# o if we encounter foo/ in $(obj-m), remove it from $(obj-m)
# and add the directory to the list of dirs to descend into: $(subdir-m)
+# make sure '/' follows subdirs
+subdir-y := $(patsubst %//,%/, $(addsuffix, /,$subdir-y))
+subdir-m
[PATCH] kbuild: implement modules.order
When multiple built-in modules (especially drivers) provide the same
capability, they're prioritized by link order specified by the order
listed in Makefile. This implicit ordering is lost for loadable
modules.
When driver modules are loaded by udev, what comes first in
modules.alias file is selected. However, the order in this file is
indeterministic (depends on filesystem listing order of installed
modules). This causes confusion.
The solution is two-parted. This patch updates kbuild such that it
generates and installs modules.order which contains the name of
modules ordered according to Makefile. The second part is update to
depmod such that it generates output files according to this file.
Note that both obj-y and obj-m subdirs can contain modules and
ordering information between those two are lost from beginning.
Currently obj-y subdirs are put before obj-m subdirs.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: Bill Nottingham [EMAIL PROTECTED]
Cc: Rusty Russell [EMAIL PROTECTED]
Cc: Greg Kroah-Hartman [EMAIL PROTECTED]
Cc: Kay Sievers [EMAIL PROTECTED]
---
Makefile |5 ++
scripts/Makefile |1
scripts/Makefile.build | 16 +++-
scripts/Makefile.lib | 10 +
scripts/remove-dup.c | 98 +
5 files changed, 128 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index a65ffd2..1c020c7 100644
--- a/Makefile
+++ b/Makefile
@@ -311,6 +311,7 @@ DEPMOD = /sbin/depmod
KALLSYMS = scripts/kallsyms
PERL = perl
CHECK = sparse
+REMOVE_DUP = scripts/remove-dup
CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise
$(CF)
MODFLAGS = -DMODULE
@@ -1023,6 +1024,7 @@ all: modules
PHONY += modules
modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux)
+ @$(REMOVE_DUP) $(vmlinux-dirs:%=$(objtree)/%/modules.order)
$(objtree)/modules.order
@echo ' Building modules, stage 2.';
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
@@ -1050,6 +1052,7 @@ _modinst_:
rm -f $(MODLIB)/build ; \
ln -s $(objtree) $(MODLIB)/build ; \
fi
+ @cp -f $(objtree)/modules.order $(MODLIB)/
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
# This depmod is only for convenience to give the initial
@@ -1109,7 +1112,7 @@ clean: archclean $(clean-dirs)
@find . $(RCS_FIND_IGNORE) \
\( -name '*.[oas]' -o -name '*.ko' -o -name '.*.cmd' \
-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
- -o -name '*.symtypes' \) \
+ -o -name '*.symtypes' -o -name 'modules.order' \) \
-type f -print | xargs rm -f
# mrproper - Delete all generated files, including .config
diff --git a/scripts/Makefile b/scripts/Makefile
index 1c73c5a..1c82547 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -12,6 +12,7 @@ hostprogs-$(CONFIG_LOGO) += pnmtologo
hostprogs-$(CONFIG_VT) += conmakehash
hostprogs-$(CONFIG_PROM_CONSOLE) += conmakehash
hostprogs-$(CONFIG_IKCONFIG) += bin2c
+hostprogs-$(CONFIG_MODULES) += remove-dup
always := $(hostprogs-y) $(hostprogs-m)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index de9836e..816a955 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -83,10 +83,12 @@ ifneq ($(strip $(obj-y) $(obj-m) $(obj-n) $(obj-)
$(lib-target)),)
builtin-target := $(obj)/built-in.o
endif
+modorder-target := $(obj)/modules.order
+
# We keep a list of all modules in $(MODVERDIR)
__build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
-$(if $(KBUILD_MODULES),$(obj-m)) \
+$(if $(KBUILD_MODULES),$(obj-m) $(modorder-target)) \
$(subdir-ym) $(always)
@:
@@ -276,6 +278,18 @@ targets += $(builtin-target)
endif # builtin-target
#
+# Rule to create modules.order file
+#
+$(modorder-target): $(subdir-ym) FORCE
+ @{ for m in $(modorder); do \
+ if echo $$m | grep -q '.*/modules.order'; then \
+ cat $$m;\
+ else\
+ echo kernel/$$m;\
+ fi; \
+ done } $@
+
+#
# Rule to compile a set of .o files into one .a file
#
ifdef lib-target
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 3c5e88b..4563c96 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -25,6 +25,15 @@ lib-y := $(filter-out $(obj-y), $(sort $(lib-y) $(lib-m)))
# o if we encounter foo/ in $(obj-m), remove it from $(obj-m)
# and add the directory to the list of dirs to descend into: $(subdir-m)
+# make sure '/' follows subdirs
+subdir-y := $(patsubst %//,%/, $(addsuffix, /,$subdir-y))
+subdir-m :=
Re: [PATCH] kbuild: implement modules.order
{snip}
Comments on your C code below.
--- /dev/null
+++ b/scripts/remove-dup.c
@@ -0,0 +1,98 @@
+/*
+ * remove-dup - Drop duplicate lines from unsorted input files
+ *
+ * Dec 2007 Tejun Heo [EMAIL PROTECTED]
+ *
+ * This software is released under GPLv2.
+ */
+
+#include stdio.h
+#include string.h
+#include stdlib.h
+
+struct hash_ent {
+ struct hash_ent *next;
+ char str[];
+};
+
+#define fatal(fmt, args...) do {\
+ fprintf(stderr, fmt , ##args); \
+ exit(1);\
+ } while (0)
+
+static inline unsigned int sdb_hash(const char *str)
+{
+unsigned int hash = 0;
+int c;
+
+while ((c = *str++))
Maybe ` while ((c = *str++) != '\0') ` is better. ;)
+ hash = c + (hash 6) + (hash 16) - hash;
+
+return hash;
+}
+
+int main(int argc, char **argv)
+{
+ unsigned int nr_entries = 0;
+ struct hash_ent **hash_tbl;
+ char line[10240];
Needs to #define the magic number?
+ int i;
+
+ /* first pass, count lines */
+ for (i = 1; i argc; i++) {
+ FILE *fp = fopen(argv[i], r);
+
+ if (!fp)
+ fatal(failed to open %s for reading\n, argv[i]);
+
+ while (fgets(line, sizeof(line), fp))
+ nr_entries++;
+
+ fclose(fp);
+ }
+
+ nr_entries = nr_entries * 10 / 8;
+ hash_tbl = calloc(nr_entries, sizeof(struct hash_ent *));
+ if (!hash_tbl)
+ fatal(failed to allocate hash table for %u entries\n,
+nr_entries);
+
+ /* second pass, hash and print unique lines */
+ for (i = 1; i argc; i++) {
+ FILE *fp = fopen(argv[i], r);
+
+ if (!fp)
+ fatal(failed to open %s for reading\n, argv[i]);
+
+ while (fgets(line, sizeof(line), fp)) {
+ int len = strlen(line);
strlen returns 'size_t', which is unsigned.
+ struct hash_ent **ppos, *new_ent;
+
+ if (line[len - 1] == '\n')
+ line[--len] = '\0';
+
+ ppos = hash_tbl + (sdb_hash(line) % nr_entries);
+ while (*ppos) {
+ if (strcmp((*ppos)-str, line) == 0)
+ break;
+ ppos = (*ppos)-next;
+ }
+ if (*ppos)
+ continue;
+
+ new_ent = malloc(sizeof(struct hash_ent) + len + 1);
+ if (!new_ent)
+ fatal(failed to allocate hash entry\n);
+ new_ent-next = NULL;
+ memcpy(new_ent-str, line, len + 1);
+
+ *ppos = new_ent;
+
+ printf(%s\n, line);
+ }
+
+ fclose(fp);
+ }
+
I think, you forgot to free(3) the memory you calloc(3)'ed and
malloc(3)'ed above.
+ return 0;
+}
Thanks!
Cong
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
WANG Cong wrote:
+static inline unsigned int sdb_hash(const char *str)
+{
+unsigned int hash = 0;
+int c;
+
+while ((c = *str++))
Maybe ` while ((c = *str++) != '\0') ` is better. ;)
Yeah, probably. That hash function is copied pasted mindlessly from web.
+hash = c + (hash 6) + (hash 16) - hash;
+
+return hash;
+}
+
+int main(int argc, char **argv)
+{
+unsigned int nr_entries = 0;
+struct hash_ent **hash_tbl;
+char line[10240];
Needs to #define the magic number?
Or maybe write a wrapper function around fgets() to detect long lines
reliably.
+while (fgets(line, sizeof(line), fp)) {
+int len = strlen(line);
strlen returns 'size_t', which is unsigned.
It's capped by the magic number above but yeah size_t would be better.
I think, you forgot to free(3) the memory you calloc(3)'ed and
malloc(3)'ed above.
It's a simple program where whole body is in main(). Why bother?
What's the benefit of adding hash-table iterating free logic?
--
tejun
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
I think, you forgot to free(3) the memory you calloc(3)'ed and malloc(3)'ed above. It's a simple program where whole body is in main(). Why bother? What's the benefit of adding hash-table iterating free logic? Personally, I think memory leaks are bugs. And we hate bugs. ;) Regards. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
WANG Cong wrote: I think, you forgot to free(3) the memory you calloc(3)'ed and malloc(3)'ed above. It's a simple program where whole body is in main(). Why bother? What's the benefit of adding hash-table iterating free logic? Personally, I think memory leaks are bugs. And we hate bugs. ;) Trust me. As a person buried alive in bug reports, I hate bugs too. I just don't agree that this type of programs should free all its resources before exiting. How about adding a comment saying /* we're going out anyway, don't bother freeing hashtable */? -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kbuild: implement modules.order
Tejun Heo wrote: WANG Cong wrote: I think, you forgot to free(3) the memory you calloc(3)'ed and malloc(3)'ed above. It's a simple program where whole body is in main(). Why bother? What's the benefit of adding hash-table iterating free logic? Personally, I think memory leaks are bugs. And we hate bugs. ;) Trust me. As a person buried alive in bug reports, I hate bugs too. I just don't agree that this type of programs should free all its resources before exiting. How about adding a comment saying /* we're going out anyway, don't bother freeing hashtable */? I don't consider this as memory leak. I agree that a comment should be enough. :) Li Zefam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
