* [PATCH] Cygwin: define byteswap.h inlines as macros
@ 2016-03-15 3:14 Yaakov Selkowitz
2016-03-15 9:03 ` Corinna Vinschen
2016-03-15 10:55 ` Václav Haisman
0 siblings, 2 replies; 9+ messages in thread
From: Yaakov Selkowitz @ 2016-03-15 3:14 UTC (permalink / raw)
To: cygwin-patches
The bswap_* "functions" are macros in glibc, so they may be tested for
by the preprocessor (e.g. #ifdef bswap_16).
Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
---
winsup/cygwin/include/byteswap.h | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/winsup/cygwin/include/byteswap.h b/winsup/cygwin/include/byteswap.h
index cd5a726..9f73c5a 100644
--- a/winsup/cygwin/include/byteswap.h
+++ b/winsup/cygwin/include/byteswap.h
@@ -16,23 +16,27 @@ extern "C" {
#endif
static __inline unsigned short
-bswap_16 (unsigned short __x)
+__bswap_16 (unsigned short __x)
{
return (__x >> 8) | (__x << 8);
}
static __inline unsigned int
-bswap_32 (unsigned int __x)
+__bswap_32 (unsigned int __x)
{
- return (bswap_16 (__x & 0xffff) << 16) | (bswap_16 (__x >> 16));
+ return (__bswap_16 (__x & 0xffff) << 16) | (__bswap_16 (__x >> 16));
}
static __inline unsigned long long
-bswap_64 (unsigned long long __x)
+__bswap_64 (unsigned long long __x)
{
- return (((unsigned long long) bswap_32 (__x & 0xffffffffull)) << 32) | (bswap_32 (__x >> 32));
+ return (((unsigned long long) __bswap_32 (__x & 0xffffffffull)) << 32) | (__bswap_32 (__x >> 32));
}
+#define bswap_16(x) __bswap_16(x)
+#define bswap_32(x) __bswap_32(x)
+#define bswap_64(x) __bswap_64(x)
+
#ifdef __cplusplus
}
#endif
--
2.7.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 3:14 [PATCH] Cygwin: define byteswap.h inlines as macros Yaakov Selkowitz
@ 2016-03-15 9:03 ` Corinna Vinschen
2016-03-15 9:15 ` Yaakov Selkowitz
2016-03-15 10:55 ` Václav Haisman
1 sibling, 1 reply; 9+ messages in thread
From: Corinna Vinschen @ 2016-03-15 9:03 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]
On Mar 14 22:13, Yaakov Selkowitz wrote:
> The bswap_* "functions" are macros in glibc, so they may be tested for
> by the preprocessor (e.g. #ifdef bswap_16).
>
> Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
> ---
> winsup/cygwin/include/byteswap.h | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/winsup/cygwin/include/byteswap.h b/winsup/cygwin/include/byteswap.h
> index cd5a726..9f73c5a 100644
> --- a/winsup/cygwin/include/byteswap.h
> +++ b/winsup/cygwin/include/byteswap.h
> @@ -16,23 +16,27 @@ extern "C" {
> #endif
>
> static __inline unsigned short
> -bswap_16 (unsigned short __x)
> +__bswap_16 (unsigned short __x)
> {
> return (__x >> 8) | (__x << 8);
> }
>
> static __inline unsigned int
> -bswap_32 (unsigned int __x)
> +__bswap_32 (unsigned int __x)
> {
> - return (bswap_16 (__x & 0xffff) << 16) | (bswap_16 (__x >> 16));
> + return (__bswap_16 (__x & 0xffff) << 16) | (__bswap_16 (__x >> 16));
> }
>
> static __inline unsigned long long
> -bswap_64 (unsigned long long __x)
> +__bswap_64 (unsigned long long __x)
> {
> - return (((unsigned long long) bswap_32 (__x & 0xffffffffull)) << 32) | (bswap_32 (__x >> 32));
> + return (((unsigned long long) __bswap_32 (__x & 0xffffffffull)) << 32) | (__bswap_32 (__x >> 32));
> }
>
> +#define bswap_16(x) __bswap_16(x)
> +#define bswap_32(x) __bswap_32(x)
> +#define bswap_64(x) __bswap_64(x)
> +
> #ifdef __cplusplus
> }
> #endif
> --
> 2.7.0
ACK.
While we're at it, what about converting the types to implicit types
(__uint16_t, __uint32_t, __uint64_t). Also, do we want to convert the
inline code to use the x86 bswap instructions?
Thanks,
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 9:03 ` Corinna Vinschen
@ 2016-03-15 9:15 ` Yaakov Selkowitz
2016-03-15 9:22 ` Corinna Vinschen
0 siblings, 1 reply; 9+ messages in thread
From: Yaakov Selkowitz @ 2016-03-15 9:15 UTC (permalink / raw)
To: cygwin-patches
On 2016-03-15 04:03, Corinna Vinschen wrote:
> On Mar 14 22:13, Yaakov Selkowitz wrote:
>> The bswap_* "functions" are macros in glibc, so they may be tested for
>> by the preprocessor (e.g. #ifdef bswap_16).
> ACK.
>
> While we're at it, what about converting the types to implicit types
> (__uint16_t, __uint32_t, __uint64_t).
glibc uses short/int/long long for these, so I think we should leave them.
> Also, do we want to convert the inline code to use the x86 bswap instructions?
Possibly, but SHTDI.
--
Yaakov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 9:15 ` Yaakov Selkowitz
@ 2016-03-15 9:22 ` Corinna Vinschen
2016-03-15 9:41 ` Yaakov Selkowitz
0 siblings, 1 reply; 9+ messages in thread
From: Corinna Vinschen @ 2016-03-15 9:22 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
On Mar 15 04:14, Yaakov Selkowitz wrote:
> On 2016-03-15 04:03, Corinna Vinschen wrote:
> >On Mar 14 22:13, Yaakov Selkowitz wrote:
> >>The bswap_* "functions" are macros in glibc, so they may be tested for
> >>by the preprocessor (e.g. #ifdef bswap_16).
> >ACK.
> >
> >While we're at it, what about converting the types to implicit types
> >(__uint16_t, __uint32_t, __uint64_t).
>
> glibc uses short/int/long long for these, so I think we should leave them.
bits/byteswap.h uses __uint64_t, but you're right for the smaller types.
> >Also, do we want to convert the inline code to use the x86 bswap instructions?
>
> Possibly, but SHTDI.
Yes.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 9:22 ` Corinna Vinschen
@ 2016-03-15 9:41 ` Yaakov Selkowitz
2016-03-15 10:04 ` Corinna Vinschen
0 siblings, 1 reply; 9+ messages in thread
From: Yaakov Selkowitz @ 2016-03-15 9:41 UTC (permalink / raw)
To: cygwin-patches
On 2016-03-15 04:22, Corinna Vinschen wrote:
> On Mar 15 04:14, Yaakov Selkowitz wrote:
>> On 2016-03-15 04:03, Corinna Vinschen wrote:
>>> On Mar 14 22:13, Yaakov Selkowitz wrote:
>>>> The bswap_* "functions" are macros in glibc, so they may be tested for
>>>> by the preprocessor (e.g. #ifdef bswap_16).
>>> ACK.
>>>
>>> While we're at it, what about converting the types to implicit types
>>> (__uint16_t, __uint32_t, __uint64_t).
>>
>> glibc uses short/int/long long for these, so I think we should leave them.
>
> bits/byteswap.h uses __uint64_t, but you're right for the smaller types.
I was looking at a cross-glibc, so that must be a recent change (unless
you're not looking at x86_64).
--
Yaakov
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 9:41 ` Yaakov Selkowitz
@ 2016-03-15 10:04 ` Corinna Vinschen
0 siblings, 0 replies; 9+ messages in thread
From: Corinna Vinschen @ 2016-03-15 10:04 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]
On Mar 15 04:41, Yaakov Selkowitz wrote:
> On 2016-03-15 04:22, Corinna Vinschen wrote:
> >On Mar 15 04:14, Yaakov Selkowitz wrote:
> >>On 2016-03-15 04:03, Corinna Vinschen wrote:
> >>>On Mar 14 22:13, Yaakov Selkowitz wrote:
> >>>>The bswap_* "functions" are macros in glibc, so they may be tested for
> >>>>by the preprocessor (e.g. #ifdef bswap_16).
> >>>ACK.
> >>>
> >>>While we're at it, what about converting the types to implicit types
> >>>(__uint16_t, __uint32_t, __uint64_t).
> >>
> >>glibc uses short/int/long long for these, so I think we should leave them.
> >
> >bits/byteswap.h uses __uint64_t, but you're right for the smaller types.
>
> I was looking at a cross-glibc, so that must be a recent change (unless
> you're not looking at x86_64).
F23 x86_64, but never mind, it's just a style issue. Personally I'd
prefer the explicitly sized types for clearness.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 3:14 [PATCH] Cygwin: define byteswap.h inlines as macros Yaakov Selkowitz
2016-03-15 9:03 ` Corinna Vinschen
@ 2016-03-15 10:55 ` Václav Haisman
2016-03-15 11:46 ` Corinna Vinschen
1 sibling, 1 reply; 9+ messages in thread
From: Václav Haisman @ 2016-03-15 10:55 UTC (permalink / raw)
To: cygwin-patches
On 15 March 2016 at 04:13, Yaakov Selkowitz <yselkowi@redhat.com> wrote:
> The bswap_* "functions" are macros in glibc, so they may be tested for
> by the preprocessor (e.g. #ifdef bswap_16).
>
> Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
> ---
> winsup/cygwin/include/byteswap.h | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/winsup/cygwin/include/byteswap.h b/winsup/cygwin/include/byteswap.h
> index cd5a726..9f73c5a 100644
> --- a/winsup/cygwin/include/byteswap.h
> +++ b/winsup/cygwin/include/byteswap.h
> @@ -16,23 +16,27 @@ extern "C" {
> #endif
>
> static __inline unsigned short
> -bswap_16 (unsigned short __x)
> +__bswap_16 (unsigned short __x)
> {
> return (__x >> 8) | (__x << 8);
> }
>
> static __inline unsigned int
> -bswap_32 (unsigned int __x)
> +__bswap_32 (unsigned int __x)
> {
> - return (bswap_16 (__x & 0xffff) << 16) | (bswap_16 (__x >> 16));
> + return (__bswap_16 (__x & 0xffff) << 16) | (__bswap_16 (__x >> 16));
> }
>
> static __inline unsigned long long
> -bswap_64 (unsigned long long __x)
> +__bswap_64 (unsigned long long __x)
> {
> - return (((unsigned long long) bswap_32 (__x & 0xffffffffull)) << 32) | (bswap_32 (__x >> 32));
> + return (((unsigned long long) __bswap_32 (__x & 0xffffffffull)) << 32) | (__bswap_32 (__x >> 32));
> }
>
> +#define bswap_16(x) __bswap_16(x)
> +#define bswap_32(x) __bswap_32(x)
> +#define bswap_64(x) __bswap_64(x)
> +
> #ifdef __cplusplus
> }
> #endif
> --
> 2.7.0
>
Would it not be better to leave the original functions as they were
and simply use these defines?
#define bswap_16 bswap_16
#define bswap_32 bswap_32
#define bswap_64 bswap_64
I believe this is valid C and C++. Untested.
--
VH
On 15 March 2016 at 04:13, Yaakov Selkowitz <yselkowi@redhat.com> wrote:
> The bswap_* "functions" are macros in glibc, so they may be tested for
> by the preprocessor (e.g. #ifdef bswap_16).
>
> Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
> ---
> winsup/cygwin/include/byteswap.h | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/winsup/cygwin/include/byteswap.h b/winsup/cygwin/include/byteswap.h
> index cd5a726..9f73c5a 100644
> --- a/winsup/cygwin/include/byteswap.h
> +++ b/winsup/cygwin/include/byteswap.h
> @@ -16,23 +16,27 @@ extern "C" {
> #endif
>
> static __inline unsigned short
> -bswap_16 (unsigned short __x)
> +__bswap_16 (unsigned short __x)
> {
> return (__x >> 8) | (__x << 8);
> }
>
> static __inline unsigned int
> -bswap_32 (unsigned int __x)
> +__bswap_32 (unsigned int __x)
> {
> - return (bswap_16 (__x & 0xffff) << 16) | (bswap_16 (__x >> 16));
> + return (__bswap_16 (__x & 0xffff) << 16) | (__bswap_16 (__x >> 16));
> }
>
> static __inline unsigned long long
> -bswap_64 (unsigned long long __x)
> +__bswap_64 (unsigned long long __x)
> {
> - return (((unsigned long long) bswap_32 (__x & 0xffffffffull)) << 32) | (bswap_32 (__x >> 32));
> + return (((unsigned long long) __bswap_32 (__x & 0xffffffffull)) << 32) | (__bswap_32 (__x >> 32));
> }
>
> +#define bswap_16(x) __bswap_16(x)
> +#define bswap_32(x) __bswap_32(x)
> +#define bswap_64(x) __bswap_64(x)
> +
> #ifdef __cplusplus
> }
> #endif
> --
> 2.7.0
>
--
VH
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 10:55 ` Václav Haisman
@ 2016-03-15 11:46 ` Corinna Vinschen
2016-03-15 15:23 ` Yaakov Selkowitz
0 siblings, 1 reply; 9+ messages in thread
From: Corinna Vinschen @ 2016-03-15 11:46 UTC (permalink / raw)
To: cygwin-patches
[-- Attachment #1: Type: text/plain, Size: 2318 bytes --]
On Mar 15 11:55, Václav Haisman wrote:
> On 15 March 2016 at 04:13, Yaakov Selkowitz <yselkowi@redhat.com> wrote:
> > The bswap_* "functions" are macros in glibc, so they may be tested for
> > by the preprocessor (e.g. #ifdef bswap_16).
> >
> > Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
> > ---
> > winsup/cygwin/include/byteswap.h | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/winsup/cygwin/include/byteswap.h b/winsup/cygwin/include/byteswap.h
> > index cd5a726..9f73c5a 100644
> > --- a/winsup/cygwin/include/byteswap.h
> > +++ b/winsup/cygwin/include/byteswap.h
> > @@ -16,23 +16,27 @@ extern "C" {
> > #endif
> >
> > static __inline unsigned short
> > -bswap_16 (unsigned short __x)
> > +__bswap_16 (unsigned short __x)
> > {
> > return (__x >> 8) | (__x << 8);
> > }
> >
> > static __inline unsigned int
> > -bswap_32 (unsigned int __x)
> > +__bswap_32 (unsigned int __x)
> > {
> > - return (bswap_16 (__x & 0xffff) << 16) | (bswap_16 (__x >> 16));
> > + return (__bswap_16 (__x & 0xffff) << 16) | (__bswap_16 (__x >> 16));
> > }
> >
> > static __inline unsigned long long
> > -bswap_64 (unsigned long long __x)
> > +__bswap_64 (unsigned long long __x)
> > {
> > - return (((unsigned long long) bswap_32 (__x & 0xffffffffull)) << 32) | (bswap_32 (__x >> 32));
> > + return (((unsigned long long) __bswap_32 (__x & 0xffffffffull)) << 32) | (__bswap_32 (__x >> 32));
> > }
> >
> > +#define bswap_16(x) __bswap_16(x)
> > +#define bswap_32(x) __bswap_32(x)
> > +#define bswap_64(x) __bswap_64(x)
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > --
> > 2.7.0
> >
>
> Would it not be better to leave the original functions as they were
> and simply use these defines?
>
> #define bswap_16 bswap_16
> #define bswap_32 bswap_32
> #define bswap_64 bswap_64
>
> I believe this is valid C and C++. Untested.
Yes, that would work. Glibc defines the inlined functions with leading
underscores as well, though. Maybe we should do the same because some
strange application wants to use the underscored versions?
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Cygwin: define byteswap.h inlines as macros
2016-03-15 11:46 ` Corinna Vinschen
@ 2016-03-15 15:23 ` Yaakov Selkowitz
0 siblings, 0 replies; 9+ messages in thread
From: Yaakov Selkowitz @ 2016-03-15 15:23 UTC (permalink / raw)
To: cygwin-patches
On 2016-03-15 06:46, Corinna Vinschen wrote:
> On Mar 15 11:55, Václav Haisman wrote:
>> Would it not be better to leave the original functions as they were
>> and simply use these defines?
>>
>> #define bswap_16 bswap_16
>> #define bswap_32 bswap_32
>> #define bswap_64 bswap_64
>>
>> I believe this is valid C and C++. Untested.
Already tried that, it leads to compiling issues at least with the
package that triggered this patch in the first place.
--
Yaakov
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-03-15 15:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-15 3:14 [PATCH] Cygwin: define byteswap.h inlines as macros Yaakov Selkowitz
2016-03-15 9:03 ` Corinna Vinschen
2016-03-15 9:15 ` Yaakov Selkowitz
2016-03-15 9:22 ` Corinna Vinschen
2016-03-15 9:41 ` Yaakov Selkowitz
2016-03-15 10:04 ` Corinna Vinschen
2016-03-15 10:55 ` Václav Haisman
2016-03-15 11:46 ` Corinna Vinschen
2016-03-15 15:23 ` Yaakov Selkowitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).