[Xcb] Win32 port

Jamey Sharp jamey at minilop.net
Wed Feb 24 00:32:03 PST 2010


On Tue, Feb 23, 2010 at 1:27 AM, Barton C Massey <bart at cs.pdx.edu> wrote:
> ... I'll merge it myself [...] unless
> somebody speaks right up right now with a *really* good
> reason not to.

I do still want review on the changes. I'd really like other folks to
look them over, especially developers on other non-Linux platforms
that we currently support.

I've gone ahead and reviewed the old patches a little, so I have some
comments now... (Yes, apparently it's possible to shame me into doing
patch review when I'm doing badly at it.)

Most importantly: There's nothing here that makes me think we can't
support Windows in XCB. I have some issues with style and other
details in places, but the fundamental changes are fine.

Why does your patch add an initWSA function to the Windows builds of
XCB? I thought it's the caller's responsibility to call WSAStartup and
WSACleanup? I don't know how that's normally handled in Windows
libraries, but at the least, the name "initWSA" doesn't fit XCB's
public naming conventions. Or maybe you meant to put initWSA in
xcbint.h rather than xcb.h, so applications can't call it?

It's a small thing, but please be careful about matching the code
style of the rest of XCB's source. github may be misleading me here,
but the whitespace looks pretty inconsistent. Maybe github handles
tabs poorly? But we don't use tabs for indentation in XCB anyway. (I
would pick a different code style if I started again, but it hardly
seems worth changing...)

We don't printf in libxcb, or if we do it's a mistake. Someday
_xcb_conn_shutdown will probably allow setting a more detailed error,
but for now just drop the debugging printf calls.

I'd feel a lot better about merging this if it didn't involve
copy/pasting some good-sized functions. One way you can cut that down
some is that I think we can replace read and write with recv and send
on Unix systems just as well as you can on Windows. Then the major
differences seem to be errno vs. WSAGetLastError(), and EWOULDBLOCK
vs. WSAEWOULDBLOCK.

This work is clearly worth cleaning up and merging. I'm sorry we
haven't done it before now.

> Maybe you can bring the port up to date one more time?

Jeetu Tweeted an "ack" to this request and I replied with a promise to
actually review it this time. :-)

Jamey


More information about the Xcb mailing list