mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Richard B. Johnson" <root@chaos.analogic.com>
To: "Hefty, Sean" <sean.hefty@intel.com>
Cc: Troy Benjegerdes <hozer@hozed.org>,
	infiniband-general@lists.sourceforge.net,
	linux-kernel@vger.kernel.org
Subject: RE: [Infiniband-general] Getting an Infiniband access layer in theLinux kernel
Date: Fri, 6 Feb 2004 12:05:55 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.53.0402061150100.3862@chaos> (raw)
In-Reply-To: <C1B7430B33A4B14F80D29B5126C5E9470326258C@orsmsx401.jf.intel.com>

On Fri, 6 Feb 2004, Hefty, Sean wrote:

> On Thu, Feb 5, 2004 at 05:11:00PM -0800, Troy Benjegerdes wrote:
> >On Thu, Feb 05, 2004 at 02:26:46PM -0800, Hefty, Sean wrote:
> >> Personally, I'm amazed that professional developers have to discuss
> or
> >> defend modular, portable code.
> >
> >You're new to linux-kernel, aren't you? ;)
>
> I was not trying to be condescending.  My point was that I think that
> everyone on this list knows the purpose and benefits behind an
> abstraction layer.  It's not something that needed to be discussed any
> further.
>
> I also understand that code in the Linux 2.6 kernel does not need
> certain abstractions.  And I agree that because we are targeting the 2.6
> kernel specifically, the existing code, some of which was developed 3-5
> years ago, should be updated based on what the 2.6 kernel provides.
>
> We want to continue to discuss specific details about what's needed to
> add the code into the kernel.  Here's a list of modifications that I
> think are needed so far:
>
> * Update the code to make direct calls for atomic operations.
> * Verify the use of spinlock calls.
> * Reformat the code for tab spacing and curly brace usage.
> * Elimination of typedefs.
>
> And, yes, knowing some of these issues up front will save the trouble of
> submitting code that will be immediately rejected.

If some major changes are being considered, I think it's time
to get rid of the:

do {  } while(0) stuff that permiates a lot of MACROS and just
use the { } as they were designed.

Before everybody screams, think. It's perfectly correct to
start a new "program unit" without a conditional expression.
You just add a curley-brace, then close the brace when you
are though.

For example (from linux/wait.h):

/*
 * Debugging macros.  We eschew `do { } while (0)' because gcc can generate
 * spurious .aligns.
 */
#if WAITQUEUE_DEBUG
#define WQ_BUG()	BUG()
#define CHECK_MAGIC(x)		\
 do {								\
		if ((x) != (long)&(x)) {			\
			printk("bad magic %lx (should be %lx), ",	\
				(long)x, (long)&(x));			\
			WQ_BUG();					\
		}							\
    } while (0)

Surely, this was some kind of work-around for some compiler bug
in a compiler you are not even allowed to use anymore for the
newer kernels.

All you need is the '{}' and none of the 'do' stuff. Since we
are still allowed to use old compilers in old kernels, I'm
nor suggesting that the headers in 2.4.xxx be changed, just the
new stuff, 2.6

Cheers,
Dick Johnson
Penguin : Linux version 2.4.24 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.



  reply	other threads:[~2004-02-06 17:04 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-02-06 16:42 Hefty, Sean
2004-02-06 17:05 ` Richard B. Johnson [this message]
2004-02-06 17:23   ` Roland Dreier
2004-02-06 18:00     ` Richard B. Johnson
2004-02-06 18:12       ` Måns Rullgård
2004-02-06 18:13       ` Chris Friesen
2004-02-06 18:22       ` Valdis.Kletnieks
2004-02-06 18:50         ` Richard B. Johnson
2004-02-06 19:02           ` Matti Aarnio
2004-02-06 19:11           ` Petr Vandrovec
2004-02-07  3:05             ` Jamie Lokier
2004-02-06 18:54         ` Måns Rullgård
2004-02-06 19:01     ` somenath
2004-02-06 17:27 ` Troy Benjegerdes
2004-02-06 18:51 ` Greg KH
2004-02-08  8:31   ` Fab Tillier
2004-02-08 16:29     ` Greg KH
2004-02-08 16:51       ` Fab Tillier
2004-02-09  2:55         ` Troy Benjegerdes
2004-02-09  2:57         ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2004-02-24 17:55 Woodruff, Robert J
2004-02-24 18:03 ` Greg KH
     [not found] <mailman.1076018705.12618.linux-kernel2news@redhat.com>
2004-02-09  1:51 ` Pete Zaitcev
2004-02-08 23:43 Arnd Bergmann
2004-02-08 21:36 Woodruff, Robert J
2004-02-06  4:07 Perez-Gonzalez, Inaky
2004-02-05 23:09 Woodruff, Robert J
2004-02-05 22:55 Woodruff, Robert J
2004-02-05 22:54 ` Randy.Dunlap
2004-02-05 22:26 Hefty, Sean
2004-02-05 22:40 ` Christoph Hellwig
2004-02-05 22:39   ` Randy.Dunlap
2004-02-05 23:19 ` Greg KH
2004-02-06  1:10 ` Troy Benjegerdes
2004-02-05 22:17 Tillier, Fabian
2004-02-05 22:56 ` Brian Gerst
2004-02-05 22:58 ` Bernd Petrovitsch
2004-02-05 22:02 Tillier, Fabian
2004-02-06  1:57 ` Troy Benjegerdes
2004-02-05 20:32 Tillier, Fabian
2004-02-05 21:27 ` Greg KH
2004-02-05 21:56   ` Chris Friesen
2004-02-06 20:22 ` Bill Davidsen
2004-02-05 19:26 Tillier, Fabian
2004-02-05 20:27 ` Greg KH

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=Pine.LNX.4.53.0402061150100.3862@chaos \
    --to=root@chaos.analogic.com \
    --cc=hozer@hozed.org \
    --cc=infiniband-general@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sean.hefty@intel.com \
    /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®