[Xcb] c_client.py: enumerator values generation

[Litvinenko, Evgeny]

Hi.

I've tried to build libxcb-1.12 (with gcc option -pedantic-errors)
and got the following error:

In file included from xinput.c:14:0:
xinput.h:3079:35: error: ISO C restricts enumerator values to range of 'int' [-Wpedantic]
XCB_INPUT_MODIFIER_MASK_ANY = 2147483648

The file xinput.h is generated during build by python script 'c_client.py'
(https://cgit.freedesktop.org/xcb/libxcb/tree/src/c_client.py)
as follows:

/usr/bin/python ./c_client.py -c "libxcb 1.12" -l "X Version 11" \
-s "3" -p /usr/lib/python2.7/site-packages \
\
/usr/share/xcb/xinput.xml

The resulted file xinput.h contains the structure that gives the error:

typedef enum xcb_input_modifier_mask_t {
XCB_INPUT_MODIFIER_MASK_ANY = 2147483648
} xcb_input_modifier_mask_t;

The structure is generated based on the followin code in xcb/proto/src/xinput.xml
(https://cgit.freedesktop.org/xcb/proto/tree/src/xinput.xml)

<enum name="ModifierMask">
<item name="Any"> <bit>31</bit> </item>
</enum>

Is it correct that xinput.h has number 2147483648 (which is 32bit long)
where as we have '<bit>31</bit>' in xinput.xml?

Thanks,
Evgeny.

________________________________

This e-mail and any attachment(s) are intended only for the recipient(s) named above and others who have been specifically authorized to receive them. They may contain confidential information. If you are not the intended recipient, please do not read this email or its attachment(s). Furthermore, you are hereby notified that any dissemination, distribution or copying of this e-mail and any attachment(s) is strictly prohibited. If you have received this e-mail in error, please immediately notify the sender by replying to this e-mail and then delete this e-mail and any attachment(s) or copies thereof from your system. Thank you.

[Litvinenko, Evgeny]

From 2a9446c38f7f1977b32f85dcf4857db8c676799c Mon Sep 17 00:00:00 2001
From: Evgeny Litvinenko <***@gmail.com>
Date: Tue, 17 Jan 2017 09:32:27 +0300
Subject: [PATCH] Convert enumerator values to the range of C 'int'

This fixes warning like the following (when gcc runs with -pedantic-errors)

In file included from xinput.c:14:0:
xinput.h:3079:35: error: ISO C restricts enumerator values to range of 'int' [-Wpedantic]
XCB_INPUT_MODIFIER_MASK_ANY = 2147483648
^

In C99 standard (ISO/IEC 9899:1999)
...
6.7.2.2 Enumeration specifiers
The expression that defines the value of an enumeration constant
shall be an integer constant expression
that has a value representable as an int.
...
---
You can use this patch or any parts of it
as you consider it necessary.

xcbgen/xtypes.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xcbgen/xtypes.py b/xcbgen/xtypes.py
index b83b119..93db2ad 100644
--- a/xcbgen/xtypes.py
+++ b/xcbgen/xtypes.py
@@ -3,6 +3,7 @@ This module contains the classes which represent XCB data types.
'''
from xcbgen.expr import Field, Expression
from xcbgen.align import Alignment, AlignmentLog
+from ctypes import c_int
import __main__

verbose_align_log = False
@@ -243,7 +244,7 @@ class Enum(SimpleType):
if value.tag == 'value':
self.values.append((item.get('name'), value.text))
elif value.tag == 'bit':
- self.values.append((item.get('name'), '%u' % (1 << int(value.text, 0))))
+ self.values.append((item.get('name'), '%i' % c_int(1 << int(value.text, 0)).value))
self.bits.append((item.get('name'), value.text))

def resolve(self, module):
--
2.11.0



Thanks,
Evgeny.

________________________________

This e-mail and any attachment(s) are intended only for the recipient(s) named above and others who have been specifically authorized to receive them. They may contain confidential information. If you are not the intended recipient, please do not read this email or its attachment(s). Furthermore, you are hereby notified that any dissemination, distribution or copying of this e-mail and any attachment(s) is strictly prohibited. If you have received this e-mail in error, please immediately notify the sender by replying to this e-mail and then delete this e-mail and any attachment(s) or copies thereof from your system. Thank you.

[Christian Linhart]

Hi Evgeny and all,

Thank you for your patch, and for bringing this issue to our attention.
However I am pretty sure we have to solve this problem in a different way.

Reasons:

1. Your proposed change is in xcb/proto which is also used by generators
for other programming languages.

They might create wrong code because the negative number may
really be interpreted in a wrong way in languages other than
C/C++.

Language specific stuff has to go into xcb/libxcb for C/C++
( or xcb/xpyb for Python, etc or whatever ).

2. I have a bad gut-feeling even for C/C++.
I am pretty sure that there are compilers which don't
interpret this negative number as the proper bit-pattern,
i.e., a 32bit value with bit 31 set.

Here's an alternative proposal for a solution:

I think that the core problem is that we abuse enums for things
that they weren't designed to, when we use them for masks.

So, instead of:

typedef enum xcb_input_modifier_mask_t {
XCB_INPUT_MODIFIER_MASK_ANY = 2147483648
} xcb_input_modifier_mask_t;

we should use something like:

typedef uint32_t xcb_input_modifier_mask_t;
static const xcb_input_modifier_mask_t XCB_INPUT_MODIFIER_MASK_ANY = 2147483648;

Instead of static const we also could use a macro if static const is not yet supported by all relevant compilers.
(for C++ we could use "enum class", but that's another story: we don't have a generator for C++ yet)

Maybe we should output the constants in masks as hex, and use the proper suffix, so that we get a
32-bit unsigned constant instead of a 64 bit signed constant:

typedef uint32_t xcb_input_modifier_mask_t;
static const xcb_input_modifier_mask_t XCB_INPUT_MODIFIER_MASK_ANY = 0x80000000U;

This would also make these constants somehow human readable.

What do you think?

Will switching from "enum" to "typedef" and "static const" cause any ABI/API issues?
I am not aware of any but maybe I overlook something.

Cheers,

Chris

P.S.:
This proposal would also allow us to define 64bit masks because we'd have control over the type, e.g.:

typedef uint64_t xcb_foo_bar_mask_t;
static const xcb_foo_bar_mask_t = XCB_FOO_BAR_BAZ 0x1ULL;
...
static const xcb_foo_bar_mask_t = XCB_FOO_BAR_ABC 0x8000000000000000ULL;

Post by Litvinenko, Evgeny
From 2a9446c38f7f1977b32f85dcf4857db8c676799c Mon Sep 17 00:00:00 2001
Date: Tue, 17 Jan 2017 09:32:27 +0300
Subject: [PATCH] Convert enumerator values to the range of C 'int'
This fixes warning like the following (when gcc runs with -pedantic-errors)
xinput.h:3079:35: error: ISO C restricts enumerator values to range of 'int' [-Wpedantic]
XCB_INPUT_MODIFIER_MASK_ANY = 2147483648
^
In C99 standard (ISO/IEC 9899:1999)
...
6.7.2.2 Enumeration specifiers
The expression that defines the value of an enumeration constant
shall be an integer constant expression
that has a value representable as an int.
...
---
You can use this patch or any parts of it
as you consider it necessary.
xcbgen/xtypes.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xcbgen/xtypes.py b/xcbgen/xtypes.py
index b83b119..93db2ad 100644
--- a/xcbgen/xtypes.py
+++ b/xcbgen/xtypes.py
@@ -3,6 +3,7 @@ This module contains the classes which represent XCB data types.
'''
from xcbgen.expr import Field, Expression
from xcbgen.align import Alignment, AlignmentLog
+from ctypes import c_int
import __main__
verbose_align_log = False
self.values.append((item.get('name'), value.text))
- self.values.append((item.get('name'), '%u' % (1 << int(value.text, 0))))
+ self.values.append((item.get('name'), '%i' % c_int(1 << int(value.text, 0)).value))
self.bits.append((item.get('name'), value.text))
--
2.11.0
Thanks,
Evgeny.
________________________________
This e-mail and any attachment(s) are intended only for the recipient(s) named above and others who have been specifically authorized to receive them. They may contain confidential information. If you are not the intended recipient, please do not read this email or its attachment(s). Furthermore, you are hereby notified that any dissemination, distribution or copying of this e-mail and any attachment(s) is strictly prohibited. If you have received this e-mail in error, please immediately notify the sender by replying to this e-mail and then delete this e-mail and any attachment(s) or copies thereof from your system. Thank you.
_______________________________________________
Xcb mailing list
https://lists.freedesktop.org/mailman/listinfo/xcb