mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mikael Pettersson <mikpe@it.uu.se>
To: "Maciej W. Rozycki" <macro@linux-mips.org>
Cc: Mikael Pettersson <mikpe@it.uu.se>, Ingo Molnar <mingo@elte.hu>,
	Chuck Ebbert <cebbert@redhat.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86: PIC, L-APIC and I/O APIC debug information
Date: Mon, 21 Jul 2008 16:04:24 +0200	[thread overview]
Message-ID: <18564.38760.212333.375520@harpo.it.uu.se> (raw)
In-Reply-To: <Pine.LNX.4.55.0807211222250.23386@cliff.in.clinika.pl>

Maciej W. Rozycki writes:
 > On Mon, 21 Jul 2008, Mikael Pettersson wrote:
 > 
 > >  > +#define __apicdebuginit(type) static type __init
 > > ...
 > >  > -void __init print_IO_APIC(void)
 > >  > +
 > >  > +__apicdebuginit(void) print_IO_APIC(void)
 > >  >  {
 > > 
 > > I _really_ dislike how this abuses the C macro preprocessor
 > > to create pointless new syntax.
 > > 
 > > Since you're editing these function definitions anyway why
 > > not just spell out "static void __init" in readable proper C?
 > 
 >  This is so that while debugging you can make all these functions global
 > with a single change in one place, rather than going through the whole
 > file and finding all the relevant function headers.  Presumably the
 > original reason for the existence of the macro.  Unfortunately an
 > object-like macro cannot be used here, as the "static" keyword has to come
 > first in a function declaration and the section attribute has to come
 > after the type designation.
 > 
 >  What's wrong with the syntax in your opinion?

It's not even remotely C-like, which will cause confusion
for anyone or anything trying to read and understand the code.

(Basically you're on the path to PL/1 or Bourne C, neither of
which were great from a software engineering point of view.)

Something like the following looks saner to me, would it work for you?

	#ifdef debug
	#define apicdebug	/* empty */
	#define __apicinit	/* empty */
	#else
	#define apicdebug	static
	#define __apicinit	__init
	#endif
	...
	apicdebug void __apicinit print_IO_APIC(void)
	{ .. }

/Mikael

  reply	other threads:[~2008-07-21 14:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-20 23:52 Maciej W. Rozycki
2008-07-21  0:36 ` Mikael Pettersson
2008-07-21 11:31   ` Maciej W. Rozycki
2008-07-21 14:04     ` Mikael Pettersson [this message]
2008-07-23  0:10       ` Maciej W. Rozycki
2008-07-24 10:37 ` Ingo Molnar

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=18564.38760.212333.375520@harpo.it.uu.se \
    --to=mikpe@it.uu.se \
    --cc=cebbert@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=macro@linux-mips.org \
    --cc=mingo@elte.hu \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®