[PATCH 1/7] byteorder: replace __u16

Jan Beulich posted 7 patches 2 months, 2 weeks ago
[PATCH 1/7] byteorder: replace __u16
Posted by Jan Beulich 2 months, 2 weeks ago
In {big,little}_endian.h the changes are entirely mechanical, except for
dealing with casting away of const from pointers-to-const on lines
touched anyway.

In swab.h the casting of constants is done away with as well - I simply
don't see what the respective comment is concerned about in our
environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
architecture, sizeof(long long) >= 8). The comment is certainly relevant
in more general cases. Excess parentheses are dropped as well,
___swab16()'s local variable is renamed, and __arch__swab16()'s is
dropped as being redundant with ___swab16()'s.

With that no uses of the type remain, so it moves to linux-compat.h.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm unconvinced of the need of the separate ___constant_swab16(). I'm
also unconvinced of the need for said constants (that even had casts on
them).

--- a/xen/include/xen/byteorder/big_endian.h
+++ b/xen/include/xen/byteorder/big_endian.h
@@ -16,25 +16,25 @@
 #define __constant_cpu_to_le32(x) ((__force __le32)___constant_swab32((x)))
 #define __constant_le32_to_cpu(x) ___constant_swab32((__force __u32)(__le32)(x))
 #define __constant_cpu_to_le16(x) ((__force __le16)___constant_swab16((x)))
-#define __constant_le16_to_cpu(x) ___constant_swab16((__force __u16)(__le16)(x))
+#define __constant_le16_to_cpu(x) ___constant_swab16((__force uint16_t)(__le16)(x))
 #define __constant_cpu_to_be64(x) ((__force __be64)(__u64)(x))
 #define __constant_be64_to_cpu(x) ((__force __u64)(__be64)(x))
 #define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x))
 #define __constant_be32_to_cpu(x) ((__force __u32)(__be32)(x))
-#define __constant_cpu_to_be16(x) ((__force __be16)(__u16)(x))
-#define __constant_be16_to_cpu(x) ((__force __u16)(__be16)(x))
+#define __constant_cpu_to_be16(x) ((__force __be16)(uint16_t)(x))
+#define __constant_be16_to_cpu(x) ((__force uint16_t)(__be16)(x))
 #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
 #define __le64_to_cpu(x) __swab64((__force __u64)(__le64)(x))
 #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
 #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
 #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
-#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
+#define __le16_to_cpu(x) __swab16((__force uint16_t)(__le16)(x))
 #define __cpu_to_be64(x) ((__force __be64)(__u64)(x))
 #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
 #define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
 #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
-#define __cpu_to_be16(x) ((__force __be16)(__u16)(x))
-#define __be16_to_cpu(x) ((__force __u16)(__be16)(x))
+#define __cpu_to_be16(x) ((__force __be16)(uint16_t)(x))
+#define __be16_to_cpu(x) ((__force uint16_t)(__be16)(x))
 
 static inline __le64 __cpu_to_le64p(const __u64 *p)
 {
@@ -52,13 +52,13 @@ static inline __u32 __le32_to_cpup(const
 {
     return __swab32p((__u32 *)p);
 }
-static inline __le16 __cpu_to_le16p(const __u16 *p)
+static inline __le16 __cpu_to_le16p(const uint16_t *p)
 {
     return (__force __le16)__swab16p(p);
 }
-static inline __u16 __le16_to_cpup(const __le16 *p)
+static inline uint16_t __le16_to_cpup(const __le16 *p)
 {
-    return __swab16p((__u16 *)p);
+    return __swab16p((const uint16_t *)p);
 }
 static inline __be64 __cpu_to_be64p(const __u64 *p)
 {
@@ -76,13 +76,13 @@ static inline __u32 __be32_to_cpup(const
 {
     return (__force __u32)*p;
 }
-static inline __be16 __cpu_to_be16p(const __u16 *p)
+static inline __be16 __cpu_to_be16p(const uint16_t *p)
 {
     return (__force __be16)*p;
 }
-static inline __u16 __be16_to_cpup(const __be16 *p)
+static inline uint16_t __be16_to_cpup(const __be16 *p)
 {
-    return (__force __u16)*p;
+    return (__force uint16_t)*p;
 }
 #define __cpu_to_le64s(x) __swab64s((x))
 #define __le64_to_cpus(x) __swab64s((x))
--- a/xen/include/xen/byteorder/little_endian.h
+++ b/xen/include/xen/byteorder/little_endian.h
@@ -15,26 +15,26 @@
 #define __constant_le64_to_cpu(x) ((__force __u64)(__le64)(x))
 #define __constant_cpu_to_le32(x) ((__force __le32)(__u32)(x))
 #define __constant_le32_to_cpu(x) ((__force __u32)(__le32)(x))
-#define __constant_cpu_to_le16(x) ((__force __le16)(__u16)(x))
-#define __constant_le16_to_cpu(x) ((__force __u16)(__le16)(x))
+#define __constant_cpu_to_le16(x) ((__force __le16)(uint16_t)(x))
+#define __constant_le16_to_cpu(x) ((__force uint16_t)(__le16)(x))
 #define __constant_cpu_to_be64(x) ((__force __be64)___constant_swab64((x)))
 #define __constant_be64_to_cpu(x) ___constant_swab64((__force __u64)(__be64)(x))
 #define __constant_cpu_to_be32(x) ((__force __be32)___constant_swab32((x)))
 #define __constant_be32_to_cpu(x) ___constant_swab32((__force __u32)(__be32)(x))
 #define __constant_cpu_to_be16(x) ((__force __be16)___constant_swab16((x)))
-#define __constant_be16_to_cpu(x) ___constant_swab16((__force __u16)(__be16)(x))
+#define __constant_be16_to_cpu(x) ___constant_swab16((__force uint16_t)(__be16)(x))
 #define __cpu_to_le64(x) ((__force __le64)(__u64)(x))
 #define __le64_to_cpu(x) ((__force __u64)(__le64)(x))
 #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
 #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
-#define __cpu_to_le16(x) ((__force __le16)(__u16)(x))
-#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
+#define __cpu_to_le16(x) ((__force __le16)(uint16_t)(x))
+#define __le16_to_cpu(x) ((__force uint16_t)(__le16)(x))
 #define __cpu_to_be64(x) ((__force __be64)__swab64((x)))
 #define __be64_to_cpu(x) __swab64((__force __u64)(__be64)(x))
 #define __cpu_to_be32(x) ((__force __be32)__swab32((x)))
 #define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x))
 #define __cpu_to_be16(x) ((__force __be16)__swab16((x)))
-#define __be16_to_cpu(x) __swab16((__force __u16)(__be16)(x))
+#define __be16_to_cpu(x) __swab16((__force uint16_t)(__be16)(x))
 
 static inline __le64 __cpu_to_le64p(const __u64 *p)
 {
@@ -52,13 +52,13 @@ static inline __u32 __le32_to_cpup(const
 {
     return (__force __u32)*p;
 }
-static inline __le16 __cpu_to_le16p(const __u16 *p)
+static inline __le16 __cpu_to_le16p(const uint16_t *p)
 {
     return (__force __le16)*p;
 }
-static inline __u16 __le16_to_cpup(const __le16 *p)
+static inline uint16_t __le16_to_cpup(const __le16 *p)
 {
-    return (__force __u16)*p;
+    return (__force uint16_t)*p;
 }
 static inline __be64 __cpu_to_be64p(const __u64 *p)
 {
@@ -76,13 +76,13 @@ static inline __u32 __be32_to_cpup(const
 {
     return __swab32p((__u32 *)p);
 }
-static inline __be16 __cpu_to_be16p(const __u16 *p)
+static inline __be16 __cpu_to_be16p(const uint16_t *p)
 {
     return (__force __be16)__swab16p(p);
 }
-static inline __u16 __be16_to_cpup(const __be16 *p)
+static inline uint16_t __be16_to_cpup(const __be16 *p)
 {
-    return __swab16p((__u16 *)p);
+    return __swab16p((const uint16_t *)p);
 }
 #define __cpu_to_le64s(x) do {} while (0)
 #define __le64_to_cpus(x) do {} while (0)
--- a/xen/include/xen/byteorder/swab.h
+++ b/xen/include/xen/byteorder/swab.h
@@ -10,15 +10,16 @@
  *    to clean up support for bizarre-endian architectures.
  */
 
-/* casts are necessary for constants, because we never know how for sure
- * how U/UL/ULL map to __u16, __u32, __u64. At least not in a portable way.
+/*
+ * Casts are necessary for constants, because we never know for sure how
+ * U/UL/ULL map to __u32, __u64. At least not in a portable way.
  */
 #define ___swab16(x)                                    \
 ({                                                      \
-    __u16 __x = (x);                                    \
-    ((__u16)(                                           \
-        (((__u16)(__x) & (__u16)0x00ffU) << 8) |        \
-        (((__u16)(__x) & (__u16)0xff00U) >> 8) ));      \
+    uint16_t x_ = (x);                                  \
+    (uint16_t)(                                         \
+        (((uint16_t)(x_) & 0x00ffU) << 8) |             \
+        (((uint16_t)(x_) & 0xff00U) >> 8));             \
 })
 
 #define ___swab32(x)                                            \
@@ -46,9 +47,9 @@
 })
 
 #define ___constant_swab16(x)                   \
-    ((__u16)(                                   \
-        (((__u16)(x) & (__u16)0x00ffU) << 8) |  \
-        (((__u16)(x) & (__u16)0xff00U) >> 8) ))
+    ((uint16_t)(                                \
+        (((uint16_t)(x) & 0x00ffU) << 8) |      \
+        (((uint16_t)(x) & 0xff00U) >> 8)))
 #define ___constant_swab32(x)                           \
     ((__u32)(                                           \
         (((__u32)(x) & (__u32)0x000000ffUL) << 24) |    \
@@ -70,7 +71,7 @@
  * provide defaults when no architecture-specific optimization is detected
  */
 #ifndef __arch__swab16
-#  define __arch__swab16(x) ({ __u16 __tmp = (x) ; ___swab16(__tmp); })
+#  define __arch__swab16(x) ___swab16(x)
 #endif
 #ifndef __arch__swab32
 #  define __arch__swab32(x) ({ __u32 __tmp = (x) ; ___swab32(__tmp); })
@@ -105,7 +106,7 @@
  */
 #if defined(__GNUC__) && defined(__OPTIMIZE__)
 #  define __swab16(x) \
-(__builtin_constant_p((__u16)(x)) ? \
+(__builtin_constant_p((uint16_t)(x)) ? \
  ___swab16((x)) : \
  __fswab16((x)))
 #  define __swab32(x) \
@@ -123,15 +124,15 @@
 #endif /* OPTIMIZE */
 
 
-static inline attr_const __u16 __fswab16(__u16 x)
+static inline attr_const uint16_t __fswab16(uint16_t x)
 {
     return __arch__swab16(x);
 }
-static inline __u16 __swab16p(const __u16 *x)
+static inline uint16_t __swab16p(const uint16_t *x)
 {
     return __arch__swab16p(x);
 }
-static inline void __swab16s(__u16 *addr)
+static inline void __swab16s(uint16_t *addr)
 {
     __arch__swab16s(addr);
 }
--- a/xen/include/xen/linux-compat.h
+++ b/xen/include/xen/linux-compat.h
@@ -14,6 +14,7 @@
 typedef int8_t  s8, __s8;
 typedef uint8_t __u8;
 typedef int16_t s16, __s16;
+typedef uint16_t __u16;
 typedef int32_t s32, __s32;
 typedef int64_t s64, __s64;
 
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -6,7 +6,7 @@
 
 /* Linux inherited types which are being phased out */
 typedef uint8_t u8;
-typedef uint16_t u16, __u16;
+typedef uint16_t u16;
 typedef uint32_t u32, __u32;
 typedef uint64_t u64, __u64;
 
@@ -51,8 +51,8 @@ typedef __PTRDIFF_TYPE__ ptrdiff_t;
 #define LONG_MIN        (-LONG_MAX - 1)
 #define ULONG_MAX       (~0UL)
 
-typedef __u16 __le16;
-typedef __u16 __be16;
+typedef uint16_t __le16;
+typedef uint16_t __be16;
 typedef __u32 __le32;
 typedef __u32 __be32;
 typedef __u64 __le64;
Re: [PATCH 1/7] byteorder: replace __u16
Posted by Andrew Cooper 2 months, 1 week ago
On 09/10/2024 10:21 am, Jan Beulich wrote:
> In {big,little}_endian.h the changes are entirely mechanical, except for
> dealing with casting away of const from pointers-to-const on lines
> touched anyway.
>
> In swab.h the casting of constants is done away with as well - I simply
> don't see what the respective comment is concerned about in our
> environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
> architecture, sizeof(long long) >= 8). The comment is certainly relevant
> in more general cases. Excess parentheses are dropped as well,
> ___swab16()'s local variable is renamed, and __arch__swab16()'s is
> dropped as being redundant with ___swab16()'s.
>
> With that no uses of the type remain, so it moves to linux-compat.h.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> I'm unconvinced of the need of the separate ___constant_swab16(). I'm
> also unconvinced of the need for said constants (that even had casts on
> them).

There is a still-good series deleting the whole of byteorder/ and
replacing it with a few-hundred line single header.

It is the second thing stalled on a governance change (prohibited
reasons to object to a change) which clearly no-one gives a damn about
fixing.  In fact double spite because it denied a good engineer his
first changes in Xen.


I don't particularly feel like trying to polish byteorder.  I'm inclined
to rebase+repost Lin's patches, at which point the majority of this
series simply disappears.

~Andrew

Re: [PATCH 1/7] byteorder: replace __u16
Posted by Jan Beulich 2 months, 1 week ago
On 09.10.2024 15:20, Andrew Cooper wrote:
> On 09/10/2024 10:21 am, Jan Beulich wrote:
>> In {big,little}_endian.h the changes are entirely mechanical, except for
>> dealing with casting away of const from pointers-to-const on lines
>> touched anyway.
>>
>> In swab.h the casting of constants is done away with as well - I simply
>> don't see what the respective comment is concerned about in our
>> environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
>> architecture, sizeof(long long) >= 8). The comment is certainly relevant
>> in more general cases. Excess parentheses are dropped as well,
>> ___swab16()'s local variable is renamed, and __arch__swab16()'s is
>> dropped as being redundant with ___swab16()'s.
>>
>> With that no uses of the type remain, so it moves to linux-compat.h.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> I'm unconvinced of the need of the separate ___constant_swab16(). I'm
>> also unconvinced of the need for said constants (that even had casts on
>> them).
> 
> There is a still-good series deleting the whole of byteorder/ and
> replacing it with a few-hundred line single header.
> 
> It is the second thing stalled on a governance change (prohibited
> reasons to object to a change) which clearly no-one gives a damn about
> fixing.  In fact double spite because it denied a good engineer his
> first changes in Xen.
> 
> 
> I don't particularly feel like trying to polish byteorder.  I'm inclined
> to rebase+repost Lin's patches, at which point the majority of this
> series simply disappears.

I wouldn't mind you doing so, as long as that other series then progresses.
What I don't want to get into is the other series being stuck rendering this
one stuck, too. Then it would imo be better to take this one first, rebase
the other on top, and work towards it becoming unstuck (whatever that takes;
I have no recollection of what the issue was back at the time, all I recall
is that, yes, there was such work at some point).

Jan

Re: [PATCH 1/7] byteorder: replace __u16
Posted by Jan Beulich 2 months, 1 week ago
On 09.10.2024 15:34, Jan Beulich wrote:
> On 09.10.2024 15:20, Andrew Cooper wrote:
>> On 09/10/2024 10:21 am, Jan Beulich wrote:
>>> In {big,little}_endian.h the changes are entirely mechanical, except for
>>> dealing with casting away of const from pointers-to-const on lines
>>> touched anyway.
>>>
>>> In swab.h the casting of constants is done away with as well - I simply
>>> don't see what the respective comment is concerned about in our
>>> environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
>>> architecture, sizeof(long long) >= 8). The comment is certainly relevant
>>> in more general cases. Excess parentheses are dropped as well,
>>> ___swab16()'s local variable is renamed, and __arch__swab16()'s is
>>> dropped as being redundant with ___swab16()'s.
>>>
>>> With that no uses of the type remain, so it moves to linux-compat.h.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> I'm unconvinced of the need of the separate ___constant_swab16(). I'm
>>> also unconvinced of the need for said constants (that even had casts on
>>> them).
>>
>> There is a still-good series deleting the whole of byteorder/ and
>> replacing it with a few-hundred line single header.
>>
>> It is the second thing stalled on a governance change (prohibited
>> reasons to object to a change) which clearly no-one gives a damn about
>> fixing.  In fact double spite because it denied a good engineer his
>> first changes in Xen.
>>
>>
>> I don't particularly feel like trying to polish byteorder.  I'm inclined
>> to rebase+repost Lin's patches, at which point the majority of this
>> series simply disappears.
> 
> I wouldn't mind you doing so, as long as that other series then progresses.
> What I don't want to get into is the other series being stuck rendering this
> one stuck, too. Then it would imo be better to take this one first, rebase
> the other on top, and work towards it becoming unstuck (whatever that takes;
> I have no recollection of what the issue was back at the time, all I recall
> is that, yes, there was such work at some point).

Just to have a clear picture: Was your reply an objection, with you indeed
meaning me to hold back this tidying work? If so, can you please indicate
when, at least roughly, you mean to re-post what you think wants re-posting?
If not, can you please indicate so, for me to commit stuff that's otherwise
ready to go in (and which that other work should be easy to re-base over)?

Jan

Re: [PATCH 1/7] byteorder: replace __u16
Posted by Jan Beulich 1 month, 3 weeks ago
On 16.10.2024 11:37, Jan Beulich wrote:
> On 09.10.2024 15:34, Jan Beulich wrote:
>> On 09.10.2024 15:20, Andrew Cooper wrote:
>>> On 09/10/2024 10:21 am, Jan Beulich wrote:
>>>> In {big,little}_endian.h the changes are entirely mechanical, except for
>>>> dealing with casting away of const from pointers-to-const on lines
>>>> touched anyway.
>>>>
>>>> In swab.h the casting of constants is done away with as well - I simply
>>>> don't see what the respective comment is concerned about in our
>>>> environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
>>>> architecture, sizeof(long long) >= 8). The comment is certainly relevant
>>>> in more general cases. Excess parentheses are dropped as well,
>>>> ___swab16()'s local variable is renamed, and __arch__swab16()'s is
>>>> dropped as being redundant with ___swab16()'s.
>>>>
>>>> With that no uses of the type remain, so it moves to linux-compat.h.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> I'm unconvinced of the need of the separate ___constant_swab16(). I'm
>>>> also unconvinced of the need for said constants (that even had casts on
>>>> them).
>>>
>>> There is a still-good series deleting the whole of byteorder/ and
>>> replacing it with a few-hundred line single header.
>>>
>>> It is the second thing stalled on a governance change (prohibited
>>> reasons to object to a change) which clearly no-one gives a damn about
>>> fixing.  In fact double spite because it denied a good engineer his
>>> first changes in Xen.
>>>
>>>
>>> I don't particularly feel like trying to polish byteorder.  I'm inclined
>>> to rebase+repost Lin's patches, at which point the majority of this
>>> series simply disappears.
>>
>> I wouldn't mind you doing so, as long as that other series then progresses.
>> What I don't want to get into is the other series being stuck rendering this
>> one stuck, too. Then it would imo be better to take this one first, rebase
>> the other on top, and work towards it becoming unstuck (whatever that takes;
>> I have no recollection of what the issue was back at the time, all I recall
>> is that, yes, there was such work at some point).
> 
> Just to have a clear picture: Was your reply an objection, with you indeed
> meaning me to hold back this tidying work? If so, can you please indicate
> when, at least roughly, you mean to re-post what you think wants re-posting?
> If not, can you please indicate so, for me to commit stuff that's otherwise
> ready to go in (and which that other work should be easy to re-base over)?

Just to mention here - short of an answer I'm going to commit this with the
R-b from Frediano that I've got.

Jan

Re: [PATCH 1/7] byteorder: replace __u16
Posted by Andrew Cooper 1 month, 3 weeks ago
On 31/10/2024 11:23 am, Jan Beulich wrote:
> On 16.10.2024 11:37, Jan Beulich wrote:
>> On 09.10.2024 15:34, Jan Beulich wrote:
>>> On 09.10.2024 15:20, Andrew Cooper wrote:
>>>> On 09/10/2024 10:21 am, Jan Beulich wrote:
>>>>> In {big,little}_endian.h the changes are entirely mechanical, except for
>>>>> dealing with casting away of const from pointers-to-const on lines
>>>>> touched anyway.
>>>>>
>>>>> In swab.h the casting of constants is done away with as well - I simply
>>>>> don't see what the respective comment is concerned about in our
>>>>> environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
>>>>> architecture, sizeof(long long) >= 8). The comment is certainly relevant
>>>>> in more general cases. Excess parentheses are dropped as well,
>>>>> ___swab16()'s local variable is renamed, and __arch__swab16()'s is
>>>>> dropped as being redundant with ___swab16()'s.
>>>>>
>>>>> With that no uses of the type remain, so it moves to linux-compat.h.
>>>>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>> ---
>>>>> I'm unconvinced of the need of the separate ___constant_swab16(). I'm
>>>>> also unconvinced of the need for said constants (that even had casts on
>>>>> them).
>>>> There is a still-good series deleting the whole of byteorder/ and
>>>> replacing it with a few-hundred line single header.
>>>>
>>>> It is the second thing stalled on a governance change (prohibited
>>>> reasons to object to a change) which clearly no-one gives a damn about
>>>> fixing.  In fact double spite because it denied a good engineer his
>>>> first changes in Xen.
>>>>
>>>>
>>>> I don't particularly feel like trying to polish byteorder.  I'm inclined
>>>> to rebase+repost Lin's patches, at which point the majority of this
>>>> series simply disappears.
>>> I wouldn't mind you doing so, as long as that other series then progresses.
>>> What I don't want to get into is the other series being stuck rendering this
>>> one stuck, too. Then it would imo be better to take this one first, rebase
>>> the other on top, and work towards it becoming unstuck (whatever that takes;
>>> I have no recollection of what the issue was back at the time, all I recall
>>> is that, yes, there was such work at some point).
>> Just to have a clear picture: Was your reply an objection, with you indeed
>> meaning me to hold back this tidying work? If so, can you please indicate
>> when, at least roughly, you mean to re-post what you think wants re-posting?
>> If not, can you please indicate so, for me to commit stuff that's otherwise
>> ready to go in (and which that other work should be easy to re-base over)?
> Just to mention here - short of an answer I'm going to commit this with the
> R-b from Frediano that I've got.

nack.

The reason there's even anything to do here is, in part, because you
were obstructive to Lin's series.

It wasn't only you, but the maintainers (plural) behaviour on that
series was so outrageous that it started the effort to governance to
prohibit certain classes of feedback, to make Xen a less toxic place to
contribute to.

I will get to it when I get to it.   You can use the time to reflect on
how you could have been more helpful in the past, and avoided this whole
issue.

~Andrew

Re: [PATCH 1/7] byteorder: replace __u16
Posted by Jan Beulich 1 month, 3 weeks ago
On 31.10.2024 13:20, Andrew Cooper wrote:
> On 31/10/2024 11:23 am, Jan Beulich wrote:
>> On 16.10.2024 11:37, Jan Beulich wrote:
>>> On 09.10.2024 15:34, Jan Beulich wrote:
>>>> On 09.10.2024 15:20, Andrew Cooper wrote:
>>>>> On 09/10/2024 10:21 am, Jan Beulich wrote:
>>>>>> In {big,little}_endian.h the changes are entirely mechanical, except for
>>>>>> dealing with casting away of const from pointers-to-const on lines
>>>>>> touched anyway.
>>>>>>
>>>>>> In swab.h the casting of constants is done away with as well - I simply
>>>>>> don't see what the respective comment is concerned about in our
>>>>>> environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
>>>>>> architecture, sizeof(long long) >= 8). The comment is certainly relevant
>>>>>> in more general cases. Excess parentheses are dropped as well,
>>>>>> ___swab16()'s local variable is renamed, and __arch__swab16()'s is
>>>>>> dropped as being redundant with ___swab16()'s.
>>>>>>
>>>>>> With that no uses of the type remain, so it moves to linux-compat.h.
>>>>>>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>> ---
>>>>>> I'm unconvinced of the need of the separate ___constant_swab16(). I'm
>>>>>> also unconvinced of the need for said constants (that even had casts on
>>>>>> them).
>>>>> There is a still-good series deleting the whole of byteorder/ and
>>>>> replacing it with a few-hundred line single header.
>>>>>
>>>>> It is the second thing stalled on a governance change (prohibited
>>>>> reasons to object to a change) which clearly no-one gives a damn about
>>>>> fixing.  In fact double spite because it denied a good engineer his
>>>>> first changes in Xen.
>>>>>
>>>>>
>>>>> I don't particularly feel like trying to polish byteorder.  I'm inclined
>>>>> to rebase+repost Lin's patches, at which point the majority of this
>>>>> series simply disappears.
>>>> I wouldn't mind you doing so, as long as that other series then progresses.
>>>> What I don't want to get into is the other series being stuck rendering this
>>>> one stuck, too. Then it would imo be better to take this one first, rebase
>>>> the other on top, and work towards it becoming unstuck (whatever that takes;
>>>> I have no recollection of what the issue was back at the time, all I recall
>>>> is that, yes, there was such work at some point).
>>> Just to have a clear picture: Was your reply an objection, with you indeed
>>> meaning me to hold back this tidying work? If so, can you please indicate
>>> when, at least roughly, you mean to re-post what you think wants re-posting?
>>> If not, can you please indicate so, for me to commit stuff that's otherwise
>>> ready to go in (and which that other work should be easy to re-base over)?
>> Just to mention here - short of an answer I'm going to commit this with the
>> R-b from Frediano that I've got.
> 
> nack.

Too late.

> The reason there's even anything to do here is, in part, because you
> were obstructive to Lin's series.
> 
> It wasn't only you, but the maintainers (plural) behaviour on that
> series was so outrageous that it started the effort to governance to
> prohibit certain classes of feedback, to make Xen a less toxic place to
> contribute to.
> 
> I will get to it when I get to it.   You can use the time to reflect on
> how you could have been more helpful in the past, and avoided this whole
> issue.

Do you really think that with this kind of reply you do any better than
what you complain about in my (supposed) earlier behavior? I can't help
the impression that you simply can't live with views differing from your
own in certain cases. If you have specific, reasonably objective comments
on that past communication (which I no longer have to hand), I may
certainly try to do better in the future. Blanket statements like those
above simply aren't actionable (and wording-wise close to a conduct
violation imo).

Jan

Re: [PATCH 1/7] byteorder: replace __u16
Posted by Frediano Ziglio 2 months, 1 week ago
On Wed, Oct 9, 2024 at 2:34 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.10.2024 15:20, Andrew Cooper wrote:
> > On 09/10/2024 10:21 am, Jan Beulich wrote:
> >> In {big,little}_endian.h the changes are entirely mechanical, except for
> >> dealing with casting away of const from pointers-to-const on lines
> >> touched anyway.
> >>
> >> In swab.h the casting of constants is done away with as well - I simply
> >> don't see what the respective comment is concerned about in our
> >> environment (sizeof(int) >= 4, sizeof(long) >= {4,8} depending on
> >> architecture, sizeof(long long) >= 8). The comment is certainly relevant
> >> in more general cases. Excess parentheses are dropped as well,
> >> ___swab16()'s local variable is renamed, and __arch__swab16()'s is
> >> dropped as being redundant with ___swab16()'s.
> >>
> >> With that no uses of the type remain, so it moves to linux-compat.h.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> I'm unconvinced of the need of the separate ___constant_swab16(). I'm
> >> also unconvinced of the need for said constants (that even had casts on
> >> them).
> >
> > There is a still-good series deleting the whole of byteorder/ and
> > replacing it with a few-hundred line single header.
> >
> > It is the second thing stalled on a governance change (prohibited
> > reasons to object to a change) which clearly no-one gives a damn about
> > fixing.  In fact double spite because it denied a good engineer his
> > first changes in Xen.
> >
> >
> > I don't particularly feel like trying to polish byteorder.  I'm inclined
> > to rebase+repost Lin's patches, at which point the majority of this
> > series simply disappears.
>
> I wouldn't mind you doing so, as long as that other series then progresses.
> What I don't want to get into is the other series being stuck rendering this
> one stuck, too. Then it would imo be better to take this one first, rebase
> the other on top, and work towards it becoming unstuck (whatever that takes;
> I have no recollection of what the issue was back at the time, all I recall
> is that, yes, there was such work at some point).
>
> Jan
>

I usually don't like stopping a series waiting for another series too.

In these mostly automated large changes instead of a manual rebase I
extract patches with "git format-patch", do the same automated replace
with sed/perl (like "s/\<u32\>/uint32_t/" and so on) and apply with
"git am".

My 2 cents.

Frediano