[PATCH] include: untangle events.h from the SDK headers.
Aaron Plattner
aplattner at nvidia.com
Tue Jul 28 21:38:12 PDT 2009
On Tue, Jul 28, 2009 at 08:49:21PM -0700, Peter Hutterer wrote:
> On Tue, Jul 28, 2009 at 08:11:35PM -0700, Aaron Plattner wrote:
> > Seems fine to me.
> >
> > This was in the original events.h already, but doesn't
> >
> > enum {
> > ET_KeyPress = 2,
> > [...]
> > ET_Internal = 0xFF /* First byte */
> > } EventType;
> >
> > declare a global variable named EventType?
>
> oops. yes. sorry, patch below.
>
> >
> > $ nm hw/xfree86/Xorg | grep EventType
> > 00000000007d4ea8 b EventType
> >
> > While we're at it, why does DeviceEvent use int plus a comment instead
> > of EventType for the "type" field?
>
> Patch below as well, a bit noisy thanks to the required default cases.
> Thanks for the review.
>
>
> From 92f88a503495010b6e45c195163c5426664966f7 Mon Sep 17 00:00:00 2001
> From: Peter Hutterer <peter.hutterer at who-t.net>
> Date: Wed, 29 Jul 2009 13:39:38 +1000
> Subject: [PATCH] include: fix enum EventType declaration.
>
> Having EventType after the enum declares a variable. silly me.
>
> Reported-by: Aaron Plattner
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> include/eventstr.h | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/eventstr.h b/include/eventstr.h
> index 3eefc05..e39beb9 100644
> --- a/include/eventstr.h
> +++ b/include/eventstr.h
> @@ -44,7 +44,7 @@
> * Note: Keep KeyPress to Motion aligned with the core events.
> * Keep ET_Raw* in the same order as KeyPress - Motion
> */
> -enum {
> +enum EventType {
> ET_KeyPress = 2,
> ET_KeyRelease,
> ET_ButtonPress,
> @@ -67,7 +67,7 @@ enum {
> ET_RawButtonRelease,
> ET_RawMotion,
> ET_Internal = 0xFF /* First byte */
> -} EventType;
> +};
>
> #define CHECKEVENT(ev) if (ev && ((InternalEvent*)(ev))->any.header != 0xFF) \
> FatalError("Wrong event type %d.\n", \
> --
> 1.6.3.rc1.2.g0164.dirty
Yes.
> From 59dfd2ccbe744de211dad36a8a89b266e721e2ad Mon Sep 17 00:00:00 2001
> From: Peter Hutterer <peter.hutterer at who-t.net>
> Date: Wed, 29 Jul 2009 13:45:32 +1000
> Subject: [PATCH] input: switch internal event types to enums.
>
> Use enum EventType instead of ints. This requires a load of default
> cases in various switch statements to silence compiler warnings.
>
> Reported-by: Aaron Plattner
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Xi/exevents.c | 6 ++++++
> dix/eventconvert.c | 11 ++++++++++-
> include/eventstr.h | 10 +++++-----
> 3 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/Xi/exevents.c b/Xi/exevents.c
> index 5c43266..4773c49 100644
> --- a/Xi/exevents.c
> +++ b/Xi/exevents.c
> @@ -131,6 +131,8 @@ IsPointerEvent(InternalEvent* event)
> case ET_Motion:
> /* XXX: enter/leave ?? */
> return TRUE;
> + default:
> + break;
> }
> return FALSE;
> }
> @@ -1066,6 +1068,8 @@ ProcessOtherEvent(InternalEvent *ev, DeviceIntPtr device)
> event->corestate = state;
> key = event->detail.key;
> break;
> + default:
> + break;
> }
>
> #if 0
> @@ -1120,6 +1124,8 @@ ProcessOtherEvent(InternalEvent *ev, DeviceIntPtr device)
> device->deviceGrab.grab->type == DeviceButtonPress ||
> device->deviceGrab.grab->type == XI_ButtonPress))
> deactivateDeviceGrab = TRUE;
> + default:
> + break;
> }
>
>
> diff --git a/dix/eventconvert.c b/dix/eventconvert.c
> index 943178e..866fa29 100644
> --- a/dix/eventconvert.c
> +++ b/dix/eventconvert.c
> @@ -147,6 +147,8 @@ EventToXI(InternalEvent *ev, xEvent **xi, int *count)
> *count = 0;
> *xi = NULL;
> return BadMatch;
> + default:
> + break;
> }
>
> ErrorF("[dix] EventToXI: Not implemented for %d \n", ev->any.type);
> @@ -196,7 +198,8 @@ EventToXI2(InternalEvent *ev, xEvent **xi)
> case ET_RawButtonRelease:
> case ET_RawMotion:
> return eventToRawEvent((RawDeviceEvent*)ev, xi);
> -
> + default:
> + break;
> }
>
> ErrorF("[dix] EventToXI2: Not implemented for %d \n", ev->any.type);
> @@ -247,6 +250,8 @@ eventToKeyButtonPointer(DeviceEvent *ev, xEvent **xi, int *count)
> case ET_KeyRelease: kbp->type = DeviceKeyRelease; break;
> case ET_ProximityIn: kbp->type = ProximityIn; break;
> case ET_ProximityOut: kbp->type = ProximityOut; break;
> + default:
> + break;
> }
>
> if (num_events > 1)
> @@ -518,6 +523,8 @@ GetCoreType(InternalEvent *event)
> case ET_ButtonRelease: coretype = ButtonRelease; break;
> case ET_KeyPress: coretype = KeyPress; break;
> case ET_KeyRelease: coretype = KeyRelease; break;
> + default:
> + break;
> }
> return coretype;
> }
> @@ -539,6 +546,8 @@ GetXIType(InternalEvent *event)
> case ET_KeyRelease: xitype = DeviceKeyRelease; break;
> case ET_ProximityIn: xitype = ProximityIn; break;
> case ET_ProximityOut: xitype = ProximityOut; break;
> + default:
> + break;
> }
> return xitype;
> }
> diff --git a/include/eventstr.h b/include/eventstr.h
> index e39beb9..06a57e3 100644
> --- a/include/eventstr.h
> +++ b/include/eventstr.h
> @@ -83,7 +83,7 @@ enum EventType {
> struct _DeviceEvent
> {
> unsigned char header; /**< Always ET_Internal */
> - int type; /**< One of EventType */
> + enum EventType type; /**< One of EventType */
> int length; /**< Length in bytes */
> Time time; /**< Time in ms */
> int deviceid; /**< Device to post this event for */
> @@ -136,7 +136,7 @@ struct _DeviceEvent
> struct _DeviceChangedEvent
> {
> unsigned char header; /**< Always ET_Internal */
> - int type; /**< ET_DeviceChanged */
> + enum EventType type; /**< ET_DeviceChanged */
> int length; /**< Length in bytes */
> Time time; /**< Time in ms */
> int deviceid; /**< Device whose capabilities have changed */
> @@ -177,7 +177,7 @@ struct _DeviceChangedEvent
> struct _DGAEvent
> {
> unsigned char header; /**< Always ET_Internal */
> - int type; /**< ET_DGAEvent */
> + enum EventType type; /**< ET_DGAEvent */
> int length; /**< Length in bytes */
> Time time; /**< Time in ms */
> int subtype; /**< KeyPress, KeyRelease, ButtonPress,
> @@ -196,7 +196,7 @@ struct _DGAEvent
> struct _RawDeviceEvent
> {
> unsigned char header; /**< Always ET_Internal */
> - int type; /**< ET_Raw */
> + enum EventType type; /**< ET_Raw */
> int length; /**< Length in bytes */
> Time time; /**< Time in ms */
> int deviceid; /**< Device to post this event for */
> @@ -221,7 +221,7 @@ struct _RawDeviceEvent
> union _InternalEvent {
> struct {
> unsigned char header; /**< Always ET_Internal */
> - int type; /**< One of ET_* */
> + enum EventType type; /**< One of ET_* */
> int length; /**< Length in bytes */
> Time time; /**< Time in ms. */
> } any;
> --
> 1.6.3.rc1.2.g0164.dirty
Usually I'd recommend adding cases for the extra values instead of adding
default cases so that anyone adding a new entry to the enum but failing to
update all the switch statements would introduce new warnings. For these,
though, there aren't ever going to be more valid cases and adding default
cases seems like the right thing to do. Maybe that's a sign that the
EventType enum is too broad, but that's diverging into type theory and
language design. The patch seems fine to me.
More information about the xorg-devel
mailing list