Re: [PATCH] Correct printk %pF to work on all architectures

[Linus Torvalds <torvalds@linux-foundation.org>]

On Wed, 3 Sep 2008, James Bottomley wrote:
> 
> You want me to pull the elf header files into lib/vsprintf.c and have
> something like

No.

I want you to stop polluting <linux/module.h> with total and utter crap.

Please tell my WHY the hell you have

	diff --git a/include/linux/module.h b/include/linux/module.h
	index 68e0955..a549f89 100644
	--- a/include/linux/module.h
	+++ b/include/linux/module.h
	@@ -76,6 +76,11 @@ void sort_extable(struct exception_table_entry *start,
	                  struct exception_table_entry *finish);
	 void sort_main_extable(void);
	 
	+/* function descriptor handling (if any) */
	+#ifndef dereference_function_descriptor
	+#define dereference_function_descriptor(p) (p)
	+#endif
	+
	 #ifdef MODULE
	 #define MODULE_GENERIC_TABLE(gtype,name)                       \
	 extern const struct gtype##_id __mod_##gtype##_table           \

in your patch? What the _hell_ does that have to do with "module.h"?

Why the heck don't you just have that in the ONLY place that cares, namely 
lib/vfprintf.c?

In other words, WHY did you do something as stupid and totally 
unexplainable as

	diff --git a/lib/vsprintf.c b/lib/vsprintf.c
	index d8d1d11..0c47629 100644
	--- a/lib/vsprintf.c
	+++ b/lib/vsprintf.c
	@@ -513,16 +513,6 @@ static char *string(char *buf, char *end, char *s, int field_width, int precisio
	        return buf;
	 }
	 
	-static inline void *dereference_function_descriptor(void *ptr)
	-{
	-#if defined(CONFIG_IA64) || defined(CONFIG_PPC64)
	-       void *p;
	-       if (!probe_kernel_address(ptr, p))
	-               ptr = p;
	-#endif
	-       return ptr;
	-}
	-

when that thing _used_ to be in a place where it made sense? Why didn't 
you just change that already existing code to use a #ifdef instead?

Why do you move that to <linux/module.h>? It makes no sense.

And why do you put the arch-specific defines in <asm/module.h>? That makes 
no sense either. WHY?

As far as I can tell, the _only_ reason you happened to pick 
<linux/module.h> was literally that it is the first of our header files 
that lib/vsprintf.c includes. And quite frankly, that makes no sense. 

That's about as sensible as putting it into <linux/string.h>. 

Put those things in some _sane_ place. That means:

 - default non-implementation in lib/vsprintf.c, since there is no point 
   in putting it anywhere else, when it's not used anywhere else.

 - arch-specific implementations can go into some more sensible asm header 
   file that is more relevant. Maybe <asm/processor.h>?

IOW, I'm complaining about your totally senseless and apparently random 
choice of location.

			Linus