[Xcb] PATCH: Removing macros in xcl.h and adding casts to Xlib
types
Barton C Massey
bart at cs.pdx.edu
Mon Jul 18 00:59:30 EST 2005
Travis,
While I really appreciate the effort that went into it, I
don't think this patch is a good idea.
If the cast macros in the source are confusing, I think
there are two better options: (1) move the cast function
generation into the XCB-XML, or (2) document the macros
properly. The features of macro-based generation are (a)
fixes to the generation code propagate to all the cast
functions, and (b) for the reader who does understand, this
makes it clear that the cast functions are all the same and
neatly summarizes what they are and how they are named.
Definitely bug the list before you work on something like
this; we'd be happy to discuss it with you. Thanks much, in
any case, for the thought and the submission!
Bart
In message <20050717071739.GX29210 at ruby.cat.pdx.edu> Travis wrote:
> The following patch removes the macros that define the casts from Xlib
> types to XCB types. They hide the functions that do all the
> conversions, and, for someone who isn't familiar with the code yet,
> putting those key functions in some hard to read C code isn't the best
> IMO. There isn't any performance benefit either since each macro
> expands to a static in-lined functions. Plus, I don't imagine that
> there will be many more casts added, so speeding up the chore of
> adding new ones isn't really as issue.
>
> At first, I combined the declarations and definitions on one line in
> one location, but it was lame that the functions stretched beyond
> column 80. I wanted the declarations to be succinct and uncluttered,
> so that they could be read quickly. For this reason, I declared them
> and later defined them at the bottom of the header file. From my
> reading of the standard, "Function specifiers shall be used only in
> the declaration of an identifier for a function" (6.7.4 of the C99
> draft N869, 18 January, 1999), so I left them off in the definitions.
>
> I tested the patch with Sun's Forte 7 compiler on a Sparc and gcc
> 3.4.3 on an x86 64 bit AMD machine. Both compiled it just fine, and
> with optimization turned way up, emitted no function call
> instructions.
>
> If the separation of function declarations and definitions isn't
> preferred just say so, and I'll combine them. Its all about column 80
> for me.
>
> Also, the patch add casts that will go the other way, i.e., from XCB
> types to Xlib types. I find myself needing those almost more often
> than the other way around.
>
> --
>
> Regards,
>
> Travis Spencer
>
> Index: xcl.h
> ===================================================================
> RCS file: /cvs/xlibs/X11/include/X11/xcl.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 xcl.h
> --- xcl.h 17 Mar 2004 18:06:35 -0000 1.1
> +++ xcl.h 17 Jul 2005 07:16:32 -0000
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2003 Jamey Sharp.
> +/* Copyright (C) 2005 Jamey Sharp and Travis Spencer.
> * This file is licensed under the MIT license. See the file COPYING. */
>
> #ifndef XCL_H
> @@ -11,34 +11,182 @@
> * On GCC/x86 with optimizations turned on, these compile to zero
> * instructions. */
>
> -#define XCLCASTDECL(src_t, dst_t, field) \
> - static inline XCB##dst_t XCL##dst_t(src_t src) \
> - { \
> - XCB##dst_t dst; \
> - dst.field = src; \
> - return dst; \
> - }
> -#define XCLXIDCASTDECL(src_t, dst_t) XCLCASTDECL(src_t, dst_t, xid)
> -#define XCLIDCASTDECL(src_t, dst_t) XCLCASTDECL(src_t, dst_t, id)
> -
> -XCLXIDCASTDECL(Window, WINDOW)
> -XCLXIDCASTDECL(Pixmap, PIXMAP)
> -XCLXIDCASTDECL(Cursor, CURSOR)
> -XCLXIDCASTDECL(Font, FONT)
> -XCLXIDCASTDECL(GContext, GCONTEXT)
> -XCLXIDCASTDECL(Colormap, COLORMAP)
> -XCLXIDCASTDECL(Atom, ATOM)
> -
> -/* For the union types, pick an arbitrary field of the union to hold the
> - * Xlib XID. Assumes the bit pattern is the same regardless of the field. */
> -XCLCASTDECL(Drawable, DRAWABLE, window.xid)
> -XCLCASTDECL(Font, FONTABLE, font.xid)
> -
> -XCLIDCASTDECL(VisualID, VISUALID)
> -XCLIDCASTDECL(Time, TIMESTAMP)
> -XCLIDCASTDECL(KeySym, KEYSYM)
> -XCLIDCASTDECL(KeyCode, KEYCODE)
> -XCLIDCASTDECL(CARD8, BUTTON)
> +static inline XCBWINDOW XCLWINDOW(Window src);
> +static inline XCBPIXMAP XCLPIXMAP(Pixmap src);
> +static inline XCBCURSOR XCLCURSOR(Cursor src);
> +static inline XCBFONT XCLFONT(Font src);
> +static inline XCBGCONTEXT XCLGCONTEXT(GContext src);
> +static inline XCBCOLORMAP XCLCOLORMAP(Colormap src);
> +static inline XCBATOM XCLATOM(Atom src);
> +
> +static inline XCBDRAWABLE XCLDRAWABLE(Drawable src);
> +static inline XCBFONTABLE XCLFONTABLE(Font src);
> +
> +static inline XCBVISUALID XCLVISUALID(VisualID src);
> +static inline XCBTIMESTAMP XCLTIMESTAMP(Time src);
> +static inline XCBKEYSYM XCLKEYSYM(KeySym src);
> +static inline XCBKEYCODE XCLKEYCODE(KeyCode src);
> +static inline XCBBUTTON XCLBUTTON(CARD8 src);
> +
> +/* Coercions from XCB types to Xlib types */
> +
> +static inline Window XCLWindow(XCBWINDOW src);
> +static inline Pixmap XCLPixmap(XCBPIXMAP src);
> +static inline Cursor XCLCursor(XCBCURSOR src);
> +static inline Font XCLFont(XCBFONT src);
> +static inline GContext XCLGContext(XCBGCONTEXT src);
> +static inline Colormap XCLColormap(XCBCOLORMAP src);
> +static inline Atom XCLAtom(XCBATOM src);
> +
> +static inline Drawable XCLDrawable(XCBDRAWABLE src);
> +static inline Font XCLFontable(XCBFONTABLE src);
> +
> +static inline VisualID XCLVisualId(XCBVISUALID src);
> +static inline Time XCLTime(XCBTIMESTAMP src);
> +static inline KeySym XCLKeySym(XCBKEYSYM src);
> +static inline KeyCode XCLKeyCode(XCBKEYCODE src);
> +static inline CARD8 XCLButton(XCBBUTTON src);
> +
> +/* Function definitions */
> +
> +static XCBWINDOW XCLWINDOW(Window src)
> +{
> + XCBWINDOW dst; dst.xid = src; return dst;
> +}
> +
> +static XCBPIXMAP XCLPIXMAP(Pixmap src)
> +{
> + XCBPIXMAP dst; dst.xid = src; return dst;
> +}
> +
> +static XCBCURSOR XCLCURSOR(Cursor src) {
> + XCBCURSOR dst; dst.xid = src; return dst;
> +}
> +
> +static XCBFONT XCLFONT(Font src)
> +{
> + XCBFONT dst; dst.xid = src; return dst;
> +}
> +
> +static XCBGCONTEXT XCLGCONTEXT(GContext src)
> +{
> + XCBGCONTEXT dst; dst.xid = src; return dst;
> +}
> +
> +static XCBCOLORMAP XCLCOLORMAP(Colormap src) {
> + XCBCOLORMAP dst; dst.xid = src; return dst;
> +}
> +
> +static XCBATOM XCLATOM(Atom src)
> +{
> + XCBATOM dst; dst.xid = src; return dst;
> +}
> +
> +static XCBDRAWABLE XCLDRAWABLE(Drawable src) {
> + XCBDRAWABLE dst; dst.window.xid = src; return dst;
> +}
> +
> +static XCBFONTABLE XCLFONTABLE(Font src)
> +{
> + XCBFONTABLE dst; dst.font.xid = src; return dst;
> +}
> +
> +static XCBVISUALID XCLVISUALID(VisualID src)
> +{
> + XCBVISUALID dst; dst.id = src; return dst;
> +}
> +
> +static XCBTIMESTAMP XCLTIMESTAMP(Time src)
> +{
> + XCBTIMESTAMP dst; dst.id = src; return dst;
> +}
> +
> +static XCBKEYSYM XCLKEYSYM(KeySym src)
> +{
> + XCBKEYSYM dst; dst.id = src; return dst;
> +}
> +
> +static XCBKEYCODE XCLKEYCODE(KeyCode src)
> +{
> + XCBKEYCODE dst; dst.id = src; return dst;
> +}
> +
> +static XCBBUTTON XCLBUTTON(CARD8 src)
> +{
> + XCBBUTTON dst; dst.id = src; return dst;
> +}
> +
> +
> +static Window XCLWindow(XCBWINDOW src)
> +{
> + return src.xid;
> +}
> +
> +static Pixmap XCLPixmap(XCBPIXMAP src)
> +{
> + return src.xid;
> +}
> +
> +static Cursor XCLCursor(XCBCURSOR src)
> +{
> + return src.xid;
> +}
> +
> +static Font XCLFont(XCBFONT src)
> +{
> + return src.xid;
> +}
> +
> +static GContext XCLGContext(XCBGCONTEXT src)
> +{
> + return src.xid;
> +}
> +
> +static Colormap XCLColormap(XCBCOLORMAP src)
> +{
> + return src.xid;
> +}
> +
> +static Atom XCLAtom(XCBATOM src)
> +{
> + return src.xid;
> +}
> +
> +
> +static Drawable XCLDrawable(XCBDRAWABLE src)
> +{
> + return src.window.xid;
> +}
> +
> +static Font XCLFontable(XCBFONTABLE src)
> +{
> + return src.font.xid;
> +}
> +
> +static VisualID XCLVisualId(XCBVISUALID src)
> +{
> + return src.id;
> +}
> +
> +static Time XCLTime(XCBTIMESTAMP src)
> +{
> + return src.id;
> +}
> +
> +static KeySym XCLKeySym(XCBKEYSYM src)
> +{
> + return src.id;
> +}
> +
> +static KeyCode XCLKeyCode(XCBKEYCODE src)
> +{
> + return src.id;
> +}
> +
> +static CARD8 XCLButton(XCBBUTTON src)
> +{
> + return src.id;
> +}
>
> /* xcl/display.c */
>
> _______________________________________________
> xcb mailing list
> xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
More information about the xcb
mailing list