[BUG] [2.5.49] symbol_get doesn't work

[BUG] [2.5.49] symbol_get doesn't work

[Wang, Stanley]

Hello,
I found the symbol_get()/symbol_put() didn't work on my 2.5.49 build.
I think the root cause is a wrong macro definition. The following patch
could 
fix this bug.

diff -Naur -X dontdiff linux-2.5.49/include/linux/module.h
linux-2.5.49-bugfix/include/linux/module.h
--- linux-2.5.49/include/linux/module.h	2002-11-26 16:06:36.000000000 +0800
+++ linux-2.5.49-bugfix/include/linux/module.h	2002-11-26
16:01:52.000000000 +0800
@@ -86,7 +86,7 @@
 /* Get/put a kernel symbol (calls must be symmetric) */
 void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
-#define symbol_get(x) ((typeof(&x))(__symbol_get(#x)))
+#define symbol_get(x) ((typeof(&x))(__symbol_get(x)))
 
 /* For every exported symbol, place a struct in the __ksymtab section */
 #define EXPORT_SYMBOL(sym)				\
@@ -166,7 +166,7 @@
 #ifdef CONFIG_MODULE_UNLOAD
 
 void __symbol_put(const char *symbol);
-#define symbol_put(x) __symbol_put(#x)
+#define symbol_put(x) __symbol_put(x)
 void symbol_put_addr(void *addr);
 
 /* We only need protection against local interrupts. */


Your Sincerely,
Stanley Wang 

SW Engineer, Intel Corporation.
Intel China Software Lab. 
Tel: 021-52574545 ext. 1171 
iNet: 8-752-1171 
 
Opinions expressed are those of the author and do not represent Intel
Corporation

Re: [BUG] [2.5.49] symbol_get doesn't work

[Rusty Russell]

In message <957BD1C2BF3CD411B6C500A0C944CA2601F11696@pdsmsx32.pd.intel.com> you
 write:
> Hello,
> I found the symbol_get()/symbol_put() didn't work on my 2.5.49 build.
> I think the root cause is a wrong macro definition. The following patch
> could 
> fix this bug.

Hi Wang,

	Thanks for the bug report, but I think you misunderstand.
symbol_get() does not take a string, it takes an identifier.  This
way, we can ensure the type is correct.  eg.

	/* In header somewhere. */
	extern int their_integer;

	....
		int *ptr = symbol_get(their_integer);
		if (!ptr) ...

Does that help?
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [BUG] [2.5.49] symbol_get doesn't work

[Rusty Lynch]

<snip>
> Hi Wang,
> 
> Thanks for the bug report, but I think you misunderstand.
> symbol_get() does not take a string, it takes an identifier.  This
> way, we can ensure the type is correct.  eg.
> 
> /* In header somewhere. */
> extern int their_integer;
> 
> ....
> int *ptr = symbol_get(their_integer);
> if (!ptr) ...
> 

I couldn't find a single file in the kernel that uses this.  What
would be the appropriate scenario for using this?  Is there
code in the kernel that is suppose to migrating to this?

    -rustyl 

Re: [BUG] [2.5.49] symbol_get doesn't work

[Rusty Russell]

In message <007401c2966b$7e370fc0$94d40a0a@amr.corp.intel.com> you write:
> <snip>
> > /* In header somewhere. */
> > extern int their_integer;
> > 
> > ....
> > int *ptr = symbol_get(their_integer);
> > if (!ptr) ...
> > 
> 
> I couldn't find a single file in the kernel that uses this.  What
> would be the appropriate scenario for using this?  Is there
> code in the kernel that is suppose to migrating to this?

That's because it's a new primitive.  Very few places really want to
use it, they usually just want to use the symbol directly.  However,
there are some places where such a dependency is too harsh: it's more
"if I can get this, great, otherwise I'll do something else".

Hope that helps,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [BUG] [2.5.49] symbol_get doesn't work

[Miles Bader]

Rusty Russell <rusty@rustcorp.com.au> writes:
> > > int *ptr = symbol_get(their_integer);
> > > if (!ptr) ...
>
> That's because it's a new primitive.  Very few places really want to
> use it, they usually just want to use the symbol directly.  However,
> there are some places where such a dependency is too harsh: it's more
> "if I can get this, great, otherwise I'll do something else".

I find the name a bit wierd, BTW -- it sounds like it's going to return
the _value_ of the symbol.  How about something like `symbol_addr' instead?

-Miles
-- 
`To alcohol!  The cause of, and solution to,
 all of life's problems' --Homer J. Simpson

Re: [BUG] [2.5.49] symbol_get doesn't work

[Sean Neakums]

commence  Miles Bader quotation:

> Rusty Russell <rusty@rustcorp.com.au> writes:
>> > > int *ptr = symbol_get(their_integer);
>> > > if (!ptr) ...
>>
>> That's because it's a new primitive.  Very few places really want to
>> use it, they usually just want to use the symbol directly.  However,
>> there are some places where such a dependency is too harsh: it's more
>> "if I can get this, great, otherwise I'll do something else".
>
> I find the name a bit wierd, BTW -- it sounds like it's going to return
> the _value_ of the symbol.  How about something like `symbol_addr' instead?

Surely the value of a symbol is precisely that: an address.

-- 
 /                          |
[|] Sean Neakums            |  Questions are a burden to others;
[|] <sneakums@zork.net>     |      answers a prison for oneself.
 \                          |

Re: [BUG] [2.5.49] symbol_get doesn't work

[Miles Bader]

Sean Neakums <sneakums@zork.net> writes:
> > I find the name a bit wierd, BTW -- it sounds like it's going to return
> > the _value_ of the symbol.  How about something like `symbol_addr' instead?
> 
> Surely the value of a symbol is precisely that: an address.

Perhaps; but in my mind the concept of `symbol' (in C) is sort of fuzzy,
and conflated with the objects to which they refer.  If I see

   x = symbol_get(some_variable);

it _looks_, at first glance, like it's going to return the value of
some_variable (maybe this simply indicates that I'm a moron, but anyway).

This:

   x = symbol_addr(some_variable);

is a whole lot more obvious, I think, regardless of how clued in you are.

-Miles
-- 
Come now, if we were really planning to harm you, would we be waiting here, 
 beside the path, in the very darkest part of the forest?

Re: [BUG] [2.5.49] symbol_get doesn't work

[Rusty Russell]

In message <buohee16u19.fsf@mcspd15.ucom.lsi.nec.co.jp> you write:
> Rusty Russell <rusty@rustcorp.com.au> writes:
> > > > int *ptr = symbol_get(their_integer);
> > > > if (!ptr) ...
> >
> > That's because it's a new primitive.  Very few places really want to
> > use it, they usually just want to use the symbol directly.  However,
> > there are some places where such a dependency is too harsh: it's more
> > "if I can get this, great, otherwise I'll do something else".
> 
> I find the name a bit wierd, BTW -- it sounds like it's going to return
> the _value_ of the symbol.  How about something like `symbol_addr' instead?

I agree, the names are a bit wierd (there were earlier get_symbol()
implementations I cribbed off), but the compiler should catch most
bugs (a void * variable being the exception).

And I already have a symbol_put_addr() separate from symbol_put(), so
I'd have to rethink the whole naming scheme, and it's a Friday 8)

Thanks,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

RE: [BUG] [2.5.49] symbol_get doesn't work

[Wang, Stanley]

Sorry, my last patch would cause an incompatible type warning. The following
patch could do better.

diff -Naur -X dontdiff linux-2.5.49/include/linux/module.h
linux-2.5.49-bugfix/include/linux/module.h
--- linux-2.5.49/include/linux/module.h	2002-11-27 09:23:39.000000000 +0800
+++ linux-2.5.49-bugfix/include/linux/module.h	2002-11-27
09:23:54.000000000 +0800
@@ -86,7 +86,7 @@
 /* Get/put a kernel symbol (calls must be symmetric) */
 void *__symbol_get(const char *symbol);
 void *__symbol_get_gpl(const char *symbol);
-#define symbol_get(x) ((typeof(&x))(__symbol_get(#x)))
+#define symbol_get(t,x) ((typeof(t))(__symbol_get(#x)))
 
 /* For every exported symbol, place a struct in the __ksymtab section */
 #define EXPORT_SYMBOL(sym)				\



Thanks,
Stanley

> -----Original Message-----
> From: Wang, Stanley 
> Sent: 2002年11月26日 16:14
> To: 'linux-kernel@vger.kernel.org'
> Cc: 'rusty@rustcorp.com.au'
> Subject: [BUG] [2.5.49] symbol_get doesn't work
> 
> 
> Hello,
> I found the symbol_get()/symbol_put() didn't work on my 2.5.49 build.
> I think the root cause is a wrong macro definition. The 
> following patch could 
> fix this bug.
> 
> diff -Naur -X dontdiff linux-2.5.49/include/linux/module.h 
> linux-2.5.49-bugfix/include/linux/module.h
> --- linux-2.5.49/include/linux/module.h	2002-11-26 
> 16:06:36.000000000 +0800
> +++ linux-2.5.49-bugfix/include/linux/module.h	
> 2002-11-26 16:01:52.000000000 +0800
> @@ -86,7 +86,7 @@
>  /* Get/put a kernel symbol (calls must be symmetric) */
>  void *__symbol_get(const char *symbol);
>  void *__symbol_get_gpl(const char *symbol);
> -#define symbol_get(x) ((typeof(&x))(__symbol_get(#x)))
> +#define symbol_get(x) ((typeof(&x))(__symbol_get(x)))
>  
>  /* For every exported symbol, place a struct in the 
> __ksymtab section */
>  #define EXPORT_SYMBOL(sym)				\
> @@ -166,7 +166,7 @@
>  #ifdef CONFIG_MODULE_UNLOAD
>  
>  void __symbol_put(const char *symbol);
> -#define symbol_put(x) __symbol_put(#x)
> +#define symbol_put(x) __symbol_put(x)
>  void symbol_put_addr(void *addr);
>  
>  /* We only need protection against local interrupts. */
> 
> 
> Your Sincerely,
> Stanley Wang 
> 
> SW Engineer, Intel Corporation.
> Intel China Software Lab. 
> Tel: 021-52574545 ext. 1171 
> iNet: 8-752-1171 
>  
> Opinions expressed are those of the author and do not 
> represent Intel Corporation
> 

Re: [BUG] [2.5.49] symbol_get doesn't work

[Rusty Russell]

In message <957BD1C2BF3CD411B6C500A0C944CA2601F11699@pdsmsx32.pd.intel.com> you
 write:
> Sorry, my last patch would cause an incompatible type warning. The =
> following
> patch could do better.
> 
> diff -Naur -X dontdiff linux-2.5.49/include/linux/module.h
> linux-2.5.49-bugfix/include/linux/module.h
> --- linux-2.5.49/include/linux/module.h	2002-11-27 09:23:39.000000000 =
> +0800
> +++ linux-2.5.49-bugfix/include/linux/module.h	2002-11-27
> 09:23:54.000000000 +0800
> @@ -86,7 +86,7 @@
>  /* Get/put a kernel symbol (calls must be symmetric) */
>  void *__symbol_get(const char *symbol);
>  void *__symbol_get_gpl(const char *symbol);
> -#define symbol_get(x) ((typeof(&x))(__symbol_get(#x)))
> +#define symbol_get(t,x) ((typeof(t))(__symbol_get(#x)))
> =20
>  /* For every exported symbol, place a struct in the __ksymtab section =
> */
>  #define EXPORT_SYMBOL(sym)				\

Hmm, I still don't think you are using it correctly.  You have to know
the type of the object you are grabbing, right?  Then you might as
well give it a declaration, no?  Preferably in a header somewhere.

Hope that helps,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [BUG] [2.5.49] symbol_get doesn't work

[Rusty Russell]

In message <957BD1C2BF3CD411B6C500A0C944CA2601F116A1@pdsmsx32.pd.intel.com> you
 write:
> Hi, Rusty
> Thanks for your respondence.
> You means that the purpose of using symbols_get() is just adding =
> module's
> reference count, right?
> If so, I think we don't need a pointer to be returned by symbol_get(). =
> We
> could use the symbols desired
> directly after we called symbol_get().=20
> How do you think about it ?

Hi Wang,

	No, you do *not* normally need to use symbol_get(): you just
use the symbol like normal and the loader will make sure reference
counts are adjusted etc.

	Here's an example of use, from my (completely untested) "make
mtd use symbol_get()" patch, which allows mtd to look for support for
a given command set at runtime (rather than forcing all command set
modules to be present to load at all):

================
typedef struct mtd_info *cfi_cmdset_fn_t(struct map_info *, int);
 
static struct mtd_info *unknown_cmdset(struct map_info *map, int primary)
{
	struct cfi_private *cfi = map->fldrv_priv;
	__u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID;

	printk(KERN_NOTICE "Support for command set %04X not present\n",
	       type);
	return NULL;
}

/* when we are built without module support, so we still link */
cfi_cmdset_fn_t cfi_cmdset_0001 __attribute__((weak, alias("unknown_cmdset")));
cfi_cmdset_fn_t cfi_cmdset_0002 __attribute__((weak, alias("unknown_cmdset")));
cfi_cmdset_fn_t cfi_cmdset_0003 __attribute__((weak, alias("unknown_cmdset")));

static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map, 
						  int primary)
{
	struct cfi_private *cfi = map->fldrv_priv;
	__u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID;
	cfi_cmdset_fn_t *probe_function = NULL;

	switch (type) {
	case 1:
		probe_function = symbol_get(cfi_cmdset_0001);
		break;
	case 2:
		probe_function = symbol_get(cfi_cmdset_0002);
		break;
	case 3:
		probe_function = symbol_get(cfi_cmdset_0003);
		break;
	}
	if (probe_function) {
 		struct mtd_info *mtd;
 
 		mtd = (*probe_function)(map, primary);
 		return mtd;
 	}
	return unknown_cmdset(map, primary);
 }
 
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.