[PATCH 001/002] scripts/mod/modpost.c: New option -E to load extra symbols

[PATCH 001/002] scripts/mod/modpost.c: New option -E to load extra symbols

[Richard Hacker]

This patch adds a new command line option -E to modpost, expecting a symbol
file as an argument which is read prior to symbol processing. -E can be 
supplied multiple times for as many files as is needed.

Signed-off-by: Richard Hacker <lerichi@gmx.net>

---
When building kernel modules that depend on other modules not in the main 
kernel tree, modpost complains about undefined symbols:
# make -C /path/to/linux/kernel M=/path/to/my/module
...
Building modules, stage 2.
....
WARNING: "rt_copy_buf" [/home/rich/osc_etl_rtw/osc_kmod.ko] undefined!
...etc

This situation occurs when modpost processes the new module's symbols. When
it finds symbols not exported by the mainline kernel, it issues this warning.

The patch adds a new command line option -E to modpost which expects a symbol
file as an argument. The symbols listed in this file are added to modpost's
symbol tables during startup. -E can be supplied as often as required.

This patch works together with the second patch. It introduces a new make
variable, EXTRA_SYMBOLS, which is used when calling modpost.

I hope this patch is useful and finds its way into the kernel.

o Changes from the previous attempt:
   - patches kernel version 2.6.24
   - patches for scripts/mod/modpost.c and scripts/Makefile.modpost are 
     separated
   - the argument to -E is now a single file, not a comma separated list 
     any more. To pass more than one file, -E is used as often as required
   - patch 002/002 in this series introduces the Kbuild variable EXTRA_SYMBOLS
     to scripts/Makefile.modpost that in turn calls modpost with the correct
     -E command line arguments.

--- linux-2.6.24-vanilla/scripts/mod/modpost.c	2008-02-27 21:10:24.000000000 
+0100
+++ linux-2.6.24/scripts/mod/modpost.c	2008-02-27 21:14:36.000000000 +0100
@@ -1658,8 +1658,12 @@ int main(int argc, char **argv)
 	char *dump_write = NULL;
 	int opt;
 	int err;
+	struct ext_sym_list {
+		struct ext_sym_list *next;
+		const char *file;
+	} *extsym_start = NULL, *extsym_iter;
 
-	while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
+	while ((opt = getopt(argc, argv, "i:I:E:mso:aw")) != -1) {
 		switch(opt) {
 			case 'i':
 				kernel_read = optarg;
@@ -1668,6 +1672,14 @@ int main(int argc, char **argv)
 				module_read = optarg;
 				external_module = 1;
 				break;
+			case 'E':
+				external_module = 1;
+				extsym_iter =
+					NOFAIL(malloc(sizeof(*extsym_iter)));
+				extsym_iter->next = extsym_start;
+				extsym_iter->file = optarg;
+				extsym_start = extsym_iter;
+				break;
 			case 'm':
 				modversions = 1;
 				break;
@@ -1692,6 +1704,12 @@ int main(int argc, char **argv)
 		read_dump(kernel_read, 1);
 	if (module_read)
 		read_dump(module_read, 0);
+	while (extsym_start) {
+		read_dump(extsym_start->file, 0);
+		extsym_iter = extsym_start->next;
+		free(extsym_start);
+		extsym_start = extsym_iter;
+	}
 
 	while (optind < argc) {
 		read_symbols(argv[optind++]);

Re: [PATCH 001/002] scripts/mod/modpost.c: New option -E to load extra symbols

[Sam Ravnborg]

Hi Richard.

I had your first patchqueued up for this and will soon take a look
at this update.

At first quick look it looked good and I have only two comments:
1) The variable should be named KBUILD_* to fit the general namespace used
2) The new possibility should be documented in Documentation/kbuild/modules.txt

I will get back when I have given it a closer look.

PS. Please also cc: linux-kbuild@vger.kernel.org if you submit again.

	Sam

On Thu, Feb 28, 2008 at 09:40:52AM +0100, Richard Hacker wrote:
> This patch adds a new command line option -E to modpost, expecting a symbol
> file as an argument which is read prior to symbol processing. -E can be 
> supplied multiple times for as many files as is needed.
> 
> Signed-off-by: Richard Hacker <lerichi@gmx.net>
> 
> ---
> When building kernel modules that depend on other modules not in the main 
> kernel tree, modpost complains about undefined symbols:
> # make -C /path/to/linux/kernel M=/path/to/my/module
> ...
> Building modules, stage 2.
> ....
> WARNING: "rt_copy_buf" [/home/rich/osc_etl_rtw/osc_kmod.ko] undefined!
> ...etc
> 
> This situation occurs when modpost processes the new module's symbols. When
> it finds symbols not exported by the mainline kernel, it issues this warning.
> 
> The patch adds a new command line option -E to modpost which expects a symbol
> file as an argument. The symbols listed in this file are added to modpost's
> symbol tables during startup. -E can be supplied as often as required.
> 
> This patch works together with the second patch. It introduces a new make
> variable, EXTRA_SYMBOLS, which is used when calling modpost.
> 
> I hope this patch is useful and finds its way into the kernel.
> 
> o Changes from the previous attempt:
>    - patches kernel version 2.6.24
>    - patches for scripts/mod/modpost.c and scripts/Makefile.modpost are 
>      separated
>    - the argument to -E is now a single file, not a comma separated list 
>      any more. To pass more than one file, -E is used as often as required
>    - patch 002/002 in this series introduces the Kbuild variable EXTRA_SYMBOLS
>      to scripts/Makefile.modpost that in turn calls modpost with the correct
>      -E command line arguments.
> 
> --- linux-2.6.24-vanilla/scripts/mod/modpost.c	2008-02-27 21:10:24.000000000 
> +0100
> +++ linux-2.6.24/scripts/mod/modpost.c	2008-02-27 21:14:36.000000000 +0100
> @@ -1658,8 +1658,12 @@ int main(int argc, char **argv)
>  	char *dump_write = NULL;
>  	int opt;
>  	int err;
> +	struct ext_sym_list {
> +		struct ext_sym_list *next;
> +		const char *file;
> +	} *extsym_start = NULL, *extsym_iter;
>  
> -	while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
> +	while ((opt = getopt(argc, argv, "i:I:E:mso:aw")) != -1) {
>  		switch(opt) {
>  			case 'i':
>  				kernel_read = optarg;
> @@ -1668,6 +1672,14 @@ int main(int argc, char **argv)
>  				module_read = optarg;
>  				external_module = 1;
>  				break;
> +			case 'E':
> +				external_module = 1;
> +				extsym_iter =
> +					NOFAIL(malloc(sizeof(*extsym_iter)));
> +				extsym_iter->next = extsym_start;
> +				extsym_iter->file = optarg;
> +				extsym_start = extsym_iter;
> +				break;
>  			case 'm':
>  				modversions = 1;
>  				break;
> @@ -1692,6 +1704,12 @@ int main(int argc, char **argv)
>  		read_dump(kernel_read, 1);
>  	if (module_read)
>  		read_dump(module_read, 0);
> +	while (extsym_start) {
> +		read_dump(extsym_start->file, 0);
> +		extsym_iter = extsym_start->next;
> +		free(extsym_start);
> +		extsym_start = extsym_iter;
> +	}
>  
>  	while (optind < argc) {
>  		read_symbols(argv[optind++]);

Re: [PATCH 001/002] scripts/mod/modpost.c: New option -E to load extra symbols

[Richard Hacker]

Hello Sam,

> At first quick look it looked good and I have only two comments:
> 1) The variable should be named KBUILD_* to fit the general namespace used
I wanted to use the namespace EXTRA_* to continue the line of EXTRA_CFLAGS, 
etc. that were used when building modules. Is that OK? The other names sound 
odd to me: KBUILD_SYMBOLS or KBUILD_EXTRA_SYMBOLS. If you really do not like 
EXTRA_SYMBOLS, which one do you propose?

> 2) The new possibility should be documented in
> Documentation/kbuild/modules.txt
[PATCH 002/002] in this revised series updated scripts/Makefile.modpost to use 
this new feature of modpost, and documented the change as well. Please take a 
look at it: http://lkml.org/lkml/2008/2/28/72

> PS. Please also cc: linux-kbuild@vger.kernel.org if you submit again.
Did it. (For new readers in linux-kbuild: The original patchset can be found 
at:
http://lkml.org/lkml/2008/2/28/71 for [PATCH 001/002]
http://lkml.org/lkml/2008/2/28/72 for [PATCH 002/002] )

- Richard

Re: [PATCH 001/002] scripts/mod/modpost.c: New option -E to load extra symbols

[Sam Ravnborg]

On Thu, Feb 28, 2008 at 09:40:52AM +0100, Richard Hacker wrote:
> This patch adds a new command line option -E to modpost, expecting a symbol
> file as an argument which is read prior to symbol processing. -E can be 
> supplied multiple times for as many files as is needed.
> 
> Signed-off-by: Richard Hacker <lerichi@gmx.net>

I edited the patches slightly and renamed the
variable to KBUILD_EXTRA_SYMBOLS.
Patches has been pushed and will soon appear in mainline.

Thanks,
	Sam