Hello Jamey, >> Why does your patch add an initWSA function to the Windows builds of >> XCB? I thought I had taken care of this so that WSAStartup and WSACleanup are handled by the caller. Will look and see if I'm not doing this. If you found this as part of the old patches then that's obsolete work and if I'm not mistaken the code in github shouldn't have this. >> It's a small thing, but please be careful about matching the code >> style of the rest of XCB's source. Probably my bad :) will work this out in my next update. >> We don't printf in libxcb, or if we do it's a mistake. Aargh I thought I'd got rid of this too. Need to review my code :) Will take it off if it's still there. >> 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. Yup this should prevent duplication. >> I'd really like other folks to look them over, especially developers on other non-Linux platforms >> that we currently support. I second that, would be nice hearing from other non-Linux platforms to see if this breaks anything out there. >> Jeetu Tweeted an "ack" to this request and I replied with a promise to >> actually review it this time. :-) :)...hopefully will have this sorted out soon....anyone else on this list on twitter?? Jeetu <a href="http://www.ebrain.in/" target="_blank">http://www.ebrain.in</a> - An open (GPL) platform to discover and run software off someone in the meshed network pool and use it on your own device. twitter: @0topcat0 and @ebrainpool <div class="gmail_quote"> On Wed, Feb 24, 2010 at 1:32 PM, Jamey Sharp <<a href="mailto:jamey@minilop.net">jamey@minilop.net</a>> wrote: <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> On Tue, Feb 23, 2010 at 1:27 AM, Barton C Massey <<a href="mailto:bart@cs.pdx.edu">bart@cs.pdx.edu</a>> wrote: > ... I'll merge it myself [...] unless <div class="im">> somebody speaks right up right now with a *really* good > reason not to. </div>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. <div class="im"> > Maybe you can bring the port up to date one more time? </div>Jeetu Tweeted an "ack" to this request and I replied with a promise to actually review it this time. :-) Jamey </blockquote></div>