All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Tokarev <mjt@tls.msk.ru>
To: Richard Henderson <rth@twiddle.net>,
	 Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-trivial@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Patch Tracking <patches@linaro.org>
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] bswap.h: Rename ldl_p, stl_p, etc to ldl_he_p, stl_he_p, etc
Date: Mon, 19 May 2014 11:53:30 +0400	[thread overview]
Message-ID: <5379B87A.6080509@msgid.tls.msk.ru> (raw)
In-Reply-To: <537505CD.1020105@twiddle.net>

15.05.2014 22:22, Richard Henderson wrote:
> On 05/15/2014 11:13 AM, Peter Maydell wrote:
>> On 2 May 2014 19:48, Richard Henderson <rth@twiddle.net> wrote:
>>> On 05/02/2014 10:32 AM, Peter Maydell wrote:
>>>> We have an unfortunate naming clash between the functions
>>>> ldl_p, stl_p, etc defined in bswap.h (which have semantics
>>>> "load/store in host endianness") and the #defines of the same
>>>> name in cpu-all.h (which have the semantics "load/store in
>>>> target endianness").
>>>>
>>>> Fortunately it turns out that the only users of the bswap.h
>>>> functions are all within bswap.h itself, so we can simply
>>>> rename them to include a _he_ infix for "host endianness".
>>>>
>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>> ---
>>>> Frankly I'm surprised that the only users of these functions
>>>> are the ones within bswap.h itself, but it's a lucky escape
>>>> from having to audit an enormous pile of code...
>>>>
>>>> We had talked about changing the "target-endian" accessors
>>>> to be ldl_te_p &c, but given the uses aren't tangled together
>>>> as I feared they would be, I'm not sure we can justify the
>>>> global function rename.
>>>
>>> I'm surprised too, but... good news, I guess.
>>>
>>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>>
>> Anybody care to suggest a submaintainer tree this should
>> go in via?
> 
> Trivial?  Ha, ha, only serious.

Oh well.

Okay.  I checked every macro in there, and indeed, it does not look
like these macros are used outside of bswap.h itself.  I modified
the macros to expand to syntactically-incorrect code and modified
all usages of those inside bswap.h to make it correct again, just
to verify, and did a rebuild.  Unfortunately I don't have easy
access to non-x86 hardware to try other host byte order, but this
should already be a good test.

So that should be an okay change.

So.. Applying to -trivial, thank you! :)

/mjt



WARNING: multiple messages have this Message-ID (diff)
From: Michael Tokarev <mjt@tls.msk.ru>
To: Richard Henderson <rth@twiddle.net>,
	Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-trivial@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Patch Tracking <patches@linaro.org>
Subject: Re: [Qemu-devel] [PATCH] bswap.h: Rename ldl_p, stl_p, etc to ldl_he_p, stl_he_p, etc
Date: Mon, 19 May 2014 11:53:30 +0400	[thread overview]
Message-ID: <5379B87A.6080509@msgid.tls.msk.ru> (raw)
In-Reply-To: <537505CD.1020105@twiddle.net>

15.05.2014 22:22, Richard Henderson wrote:
> On 05/15/2014 11:13 AM, Peter Maydell wrote:
>> On 2 May 2014 19:48, Richard Henderson <rth@twiddle.net> wrote:
>>> On 05/02/2014 10:32 AM, Peter Maydell wrote:
>>>> We have an unfortunate naming clash between the functions
>>>> ldl_p, stl_p, etc defined in bswap.h (which have semantics
>>>> "load/store in host endianness") and the #defines of the same
>>>> name in cpu-all.h (which have the semantics "load/store in
>>>> target endianness").
>>>>
>>>> Fortunately it turns out that the only users of the bswap.h
>>>> functions are all within bswap.h itself, so we can simply
>>>> rename them to include a _he_ infix for "host endianness".
>>>>
>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>> ---
>>>> Frankly I'm surprised that the only users of these functions
>>>> are the ones within bswap.h itself, but it's a lucky escape
>>>> from having to audit an enormous pile of code...
>>>>
>>>> We had talked about changing the "target-endian" accessors
>>>> to be ldl_te_p &c, but given the uses aren't tangled together
>>>> as I feared they would be, I'm not sure we can justify the
>>>> global function rename.
>>>
>>> I'm surprised too, but... good news, I guess.
>>>
>>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>>
>> Anybody care to suggest a submaintainer tree this should
>> go in via?
> 
> Trivial?  Ha, ha, only serious.

Oh well.

Okay.  I checked every macro in there, and indeed, it does not look
like these macros are used outside of bswap.h itself.  I modified
the macros to expand to syntactically-incorrect code and modified
all usages of those inside bswap.h to make it correct again, just
to verify, and did a rebuild.  Unfortunately I don't have easy
access to non-x86 hardware to try other host byte order, but this
should already be a good test.

So that should be an okay change.

So.. Applying to -trivial, thank you! :)

/mjt

  reply	other threads:[~2014-05-19  7:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-02 17:32 [Qemu-devel] [PATCH] bswap.h: Rename ldl_p, stl_p, etc to ldl_he_p, stl_he_p, etc Peter Maydell
2014-05-02 18:48 ` Richard Henderson
2014-05-15 18:13   ` Peter Maydell
2014-05-15 18:22     ` [Qemu-trivial] " Richard Henderson
2014-05-15 18:22       ` [Qemu-devel] " Richard Henderson
2014-05-19  7:53       ` Michael Tokarev [this message]
2014-05-19  7:53         ` Michael Tokarev

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=5379B87A.6080509@msgid.tls.msk.ru \
    --to=mjt@tls.msk.ru \
    --cc=patches@linaro.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=rth@twiddle.net \
    /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.