From: greg@kroah.com (Greg KH)
To: kernelnewbies@lists.kernelnewbies.org
Subject: parameter of module_init() and module_exit() must not be a macro
Date: Fri, 16 Oct 2015 06:59:58 -0700 [thread overview]
Message-ID: <20151016135958.GA12791@kroah.com> (raw)
In-Reply-To: <6D83E89737156549AEA25EF9ED712C5D159B91@DEFTHW99EK1MSX.ww902.siemens.net>
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
prev parent reply other threads:[~2015-10-16 13:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-15 9:48 parameter of module_init() and module_exit() must not be a macro Warlich, Christof
2015-10-15 13:55 ` Greg KH
2015-10-15 14:26 ` AW: " Warlich, Christof
2015-10-15 15:44 ` Greg KH
2015-10-16 6:40 ` AW: " Warlich, Christof
2015-10-16 13:59 ` Greg KH [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151016135958.GA12791@kroah.com \
--to=greg@kroah.com \
--cc=kernelnewbies@lists.kernelnewbies.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.