public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Matt Mackall <mpm@selenic.com>
To: David Wagner <daw@cs.berkeley.edu>
Cc: bos@serpentine.com, linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1 of 3] Introduce __memcpy_toio32
Date: Wed, 28 Dec 2005 00:20:56 -0600	[thread overview]
Message-ID: <20051228062056.GH3356@waste.org> (raw)
In-Reply-To: <200512280603.jBS63WGB031117@taverner.CS.Berkeley.EDU>

On Tue, Dec 27, 2005 at 10:03:32PM -0800, David Wagner wrote:
> 
> In article <20051228035231.GA3356@waste.org> you write:
> >> +void fastcall __memcpy_toio32(volatile void __iomem *d, const void
> >*s, size_t count)
> >> +{
> >> +	volatile u32 __iomem *dst = d;
> >> +	const u32 *src = s;
> >> +
> >> +	while (--count >= 0) {
> >> +		__raw_writel(*src++, dst++);
> >> +}
> >
> >Suspicious use of volatile - writel is doing the actual write, this
> >function never does a dereference. As you've already got private
> >copies of the pointers already in s and d, it's perfectily reasonable
> >and idiomatic to do:
> >
> >	while (--count >= 0)
> >		__raw_writel(*s++, d++);
> 
> I don't think *s++ or d++ is going to be accepted by the compiler,
> given that s and d are both "void *"'s.  The posted code casts to
> "u32 *", which should make the dereference and autoincrement work
> correctly.
> 
> [Not posted to linux-kernel, in case I'm totally confused.]

No, you're absolutely right. It's idiomatic for things like strcpy,
where the type is known, but not for memcpy.

However, it just so happens that GCC will happily treat math on void
pointers as if they were char pointers. So this in fact will
compile without errors:

void cpy(void *a, void *b, int count)
{
        while (count--)
                copybyte(a++, b++);
}

Linus thinks this is a feature but I don't, so I'm not recommending
it. In fact, given the whole point of this function is to copy dwords,
not bytes, this GCCism fails us for type-safety.

At any rate, the dereference on s above _is_ a compile error - it's void
type.

[cc:ed back to lkml to keep me honest]

-- 
Mathematics is the supreme nostalgia of our time.

       reply	other threads:[~2005-12-28  6:24 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200512280603.jBS63WGB031117@taverner.CS.Berkeley.EDU>
2005-12-28  6:20 ` Matt Mackall [this message]
2005-12-27 23:41 [PATCH 0 of 3] Add memcpy_toio32, a 32-bit MMIO copy routine Bryan O'Sullivan
2005-12-27 23:41 ` [PATCH 1 of 3] Introduce __memcpy_toio32 Bryan O'Sullivan
2005-12-28  1:10   ` Roland Dreier
2005-12-28 14:40     ` Bryan O'Sullivan
2005-12-28 14:51       ` Matt Mackall
2005-12-30 23:46         ` Adrian Bunk
2005-12-30 23:44           ` Matt Mackall
2005-12-31  0:23             ` Linus Torvalds
2005-12-31 21:24               ` Adrian Bunk
2005-12-28 19:23       ` Roland Dreier
2005-12-28  1:11   ` Roland Dreier
2005-12-28  4:07     ` Matt Mackall
2005-12-28  3:52   ` Matt Mackall
2005-12-28 14:47     ` Bryan O'Sullivan
2005-12-28 14:55       ` Matt Mackall
2005-12-28 15:18     ` Geert Uytterhoeven
2005-12-28 15:52       ` Bryan O'Sullivan

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=20051228062056.GH3356@waste.org \
    --to=mpm@selenic.com \
    --cc=bos@serpentine.com \
    --cc=daw@cs.berkeley.edu \
    --cc=linux-kernel@vger.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