From mboxrd@z Thu Jan  1 00:00:00 1970
From: greg@kroah.com (Greg KH)
Date: Fri, 16 Oct 2015 06:59:58 -0700
Subject: parameter of module_init() and module_exit() must not be a macro
In-Reply-To: <6D83E89737156549AEA25EF9ED712C5D159B91@DEFTHW99EK1MSX.ww902.siemens.net>
References: <6D83E89737156549AEA25EF9ED712C5D159818@DEFTHW99EK1MSX.ww902.siemens.net>
	<20151015135522.GA8230@kroah.com>
	<6D83E89737156549AEA25EF9ED712C5D1598FC@DEFTHW99EK1MSX.ww902.siemens.net>
	<20151015154410.GB8818@kroah.com>
	<6D83E89737156549AEA25EF9ED712C5D159B91@DEFTHW99EK1MSX.ww902.siemens.net>
Message-ID: <20151016135958.GA12791@kroah.com>
To: kernelnewbies@lists.kernelnewbies.org
List-Id: kernelnewbies.lists.kernelnewbies.org

On Fri, Oct 16, 2015 at 06:40:40AM +0000, Warlich, Christof wrote:
> > Ick, no, don't do that, you will make life much harder for everyone
> > involved in the end.  Just write out the code "for real", it's trivial
> > to do so, and you aren't really saving anyone anytime as nothing like
> > this would ever be acceptable to the upstream kernel developers.
> > 
> > Really, you aren't saving any time/energy here, the "template" code is
> > the easiest part to write, it's what is in those functions that really
> > matters.
> 
> Ok, agreed, the CONCAT() construct looks a bit strange at first sight, although
> things could further be simplified, e.g.:
> 
> #define _CONCAT(x, y) x##y
> #define CONCAT(x, y) _CONCAT(x, y)
> #define _STRINGIFY(x) #x
> #define STRINGIFY(x) _STRINGIFY(x)
> #define DRIVER_NAMESPACE(x) CONCAT(CONCAT(DRIVER_NAME, _), x)
> 
> ...
> 
> static void __exit DRIVER_NAMESPACE(exit)(void)
> {
> 	...
> }
> 
> module_init(DRIVER_NAMESPACE(init));
> module_exit(DRIVER_NAMESPACE(exit));
> 
> But anyhow, as you are most probably right that this would not be accepted
> upstream, there is not much point in arguing further whether my example is a
> good idea.
> 
> I'd still like to make a point if the current implementation of the module_init()
> and module_exit() macros is _correct_ though. With the following code snippet:
> 
> #define FOO BAR
> module_init(FOO);
> 
> the current implementation of the module_init() macro would expand to
> 
> 	static inline initcall_t __inittest(void)		\
> 	{ return BAR; }					\
> 	int init_module(void) __attribute__((alias("FOO")));
> 
> i.e. we see both FOO and BAR in the expanded code, which is rather unexpected.
> The patch that I've suggested would fix that, as the macro would then expand to:
> 
> 	static inline initcall_t __inittest(void)		\
> 	{ return BAR; }					\
> 	int init_module(void) __attribute__((alias("BAR")));
> 
> Thus, even when not being able to give an acceptable real world example, my
> proposed patch would still enforce the principle of "least surprise". Just consider
> this similar to coding standardization patches: It would just improve overall
> code quality.
> 
> So my initial question remains: How are the chances to get my (code quality
> improvement) patch upstream?

As it doesn't fix an in-kernel issues, I doubt it's worth the effort,
but I'm not the maintainer of this portion of the kernel, so I do not
know if it will be accepted or not, sorry.

good luck!

greg k-h