From: "Bradley D. LaRonde" <brad@ltc.com>
To: <jim@jtan.com>, "Justin Carlson" <justinca@ri.cmu.edu>
Cc: <linux-mips@oss.sgi.com>
Subject: Re: PATCH: io.h remove detrimental do {...} whiles, add sequence points, add const modifiers
Date: Fri, 7 Dec 2001 15:23:33 -0500 [thread overview]
Message-ID: <01b801c17f5d$0af02180$5601010a@prefect> (raw)
In-Reply-To: 20011207144343.A4417@neurosis.mit.edu
----- Original Message -----
From: "Jim Paris" <jim@jtan.com>
To: "Justin Carlson" <justinca@ri.cmu.edu>
Cc: <linux-mips@oss.sgi.com>
Sent: Friday, December 07, 2001 2:43 PM
Subject: Re: PATCH: io.h remove detrimental do {...} whiles, add sequence
points, add const modifiers
> > Maybe I missed this, but is there any reason for the patch, other then
> > a personal preference of how to do macros that look like functions?
> > I've seen gcc do strange non-optimal things with functions declared
> > inlines, but I've never seen it generate bad code WRT to do{}while(0)
> > constructs.
> >
> > Unless I'm missing something, this patch looks like a solution in search
> > of a problem...
No Justin. See below.
> In the case of set_io_port_base, I see no real reason. But for the
> out[b,w,l] functions, having the do/while can prevent constructs that
> might otherwise make sense, like
>
> for(i=0;i<10;i++,outb(i,port)) {
> ...
> }
>
> Okay, so it's a bad example, but.. :) Maybe Brad has a better one.
>From drivers/net/wireless/heremes.h:
<snip>
/* Register access convenience macros */
#define hermes_read_reg(hw, off) (inw((hw)->iobase + (off)))
#define hermes_write_reg(hw, off, val) (outw_p((val), (hw)->iobase + (off)))
#define hermes_read_regn(hw, name) (hermes_read_reg((hw), HERMES_##name))
#define hermes_write_regn(hw, name, val) (hermes_write_reg((hw),
HERMES_##name, (val)))
/* Note that for the next two, the count is in 16-bit words, not bytes */
#define hermes_read_data(hw, off, buf, count) (insw((hw)->iobase + (off),
(buf), (count)))
#define hermes_write_data(hw, off, buf, count) (outsw((hw)->iobase + (off),
(buf), (count)))
</snip>
That won't compile with the do {...} while(0)s left in io.h. My patch lets
hermes.h (and all other io code that I've tested) compile.
heremes.h compiles as-is on other platforms. Why should mips snub it for
some dubious value of do {...} while(0)?
Regards,
Brad
WARNING: multiple messages have this Message-ID (diff)
From: "Bradley D. LaRonde" <brad@ltc.com>
To: jim@jtan.com, Justin Carlson <justinca@ri.cmu.edu>
Cc: linux-mips@oss.sgi.com
Subject: Re: PATCH: io.h remove detrimental do {...} whiles, add sequence points, add const modifiers
Date: Fri, 7 Dec 2001 15:23:33 -0500 [thread overview]
Message-ID: <01b801c17f5d$0af02180$5601010a@prefect> (raw)
Message-ID: <20011207202333.xFQx5GuxlJDr4Rj5m6CnnsJt3jZBbTiecqhD4hEPtbA@z> (raw)
In-Reply-To: 20011207144343.A4417@neurosis.mit.edu
----- Original Message -----
From: "Jim Paris" <jim@jtan.com>
To: "Justin Carlson" <justinca@ri.cmu.edu>
Cc: <linux-mips@oss.sgi.com>
Sent: Friday, December 07, 2001 2:43 PM
Subject: Re: PATCH: io.h remove detrimental do {...} whiles, add sequence
points, add const modifiers
> > Maybe I missed this, but is there any reason for the patch, other then
> > a personal preference of how to do macros that look like functions?
> > I've seen gcc do strange non-optimal things with functions declared
> > inlines, but I've never seen it generate bad code WRT to do{}while(0)
> > constructs.
> >
> > Unless I'm missing something, this patch looks like a solution in search
> > of a problem...
No Justin. See below.
> In the case of set_io_port_base, I see no real reason. But for the
> out[b,w,l] functions, having the do/while can prevent constructs that
> might otherwise make sense, like
>
> for(i=0;i<10;i++,outb(i,port)) {
> ...
> }
>
> Okay, so it's a bad example, but.. :) Maybe Brad has a better one.
From drivers/net/wireless/heremes.h:
<snip>
/* Register access convenience macros */
#define hermes_read_reg(hw, off) (inw((hw)->iobase + (off)))
#define hermes_write_reg(hw, off, val) (outw_p((val), (hw)->iobase + (off)))
#define hermes_read_regn(hw, name) (hermes_read_reg((hw), HERMES_##name))
#define hermes_write_regn(hw, name, val) (hermes_write_reg((hw),
HERMES_##name, (val)))
/* Note that for the next two, the count is in 16-bit words, not bytes */
#define hermes_read_data(hw, off, buf, count) (insw((hw)->iobase + (off),
(buf), (count)))
#define hermes_write_data(hw, off, buf, count) (outsw((hw)->iobase + (off),
(buf), (count)))
</snip>
That won't compile with the do {...} while(0)s left in io.h. My patch lets
hermes.h (and all other io code that I've tested) compile.
heremes.h compiles as-is on other platforms. Why should mips snub it for
some dubious value of do {...} while(0)?
Regards,
Brad
next prev parent reply other threads:[~2001-12-07 21:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2001-12-07 17:14 PATCH: io.h remove detrimental do {...} whiles, add sequence points, add const modifiers Bradley D. LaRonde
2001-12-07 17:30 ` Geert Uytterhoeven
2001-12-07 17:38 ` Daniel Jacobowitz
2001-12-07 18:04 ` Jim Paris
2001-12-07 18:06 ` Ralf Baechle
2001-12-07 18:15 ` Jim Paris
2001-12-07 19:36 ` Justin Carlson
2001-12-07 19:43 ` Jim Paris
2001-12-07 20:23 ` Bradley D. LaRonde [this message]
2001-12-07 20:23 ` Bradley D. LaRonde
2001-12-07 20:44 ` Justin Carlson
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='01b801c17f5d$0af02180$5601010a@prefect' \
--to=brad@ltc.com \
--cc=jim@jtan.com \
--cc=justinca@ri.cmu.edu \
--cc=linux-mips@oss.sgi.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