Hi Barton, >I don't have much to contribute, I'm afraid. :-) Just big >thank-yous to Jeetu and Julien; I'm really pleased to see >this move forward. Appreciate the opportunity and glad I could help. I would like to thank Vincent, Peter and Julien for their help and of course all the rest of the devels who have put together quite a cool library :) Hopefully will put together the changes recommended by everyone soon. Bye for now <div class="gmail_quote">On Wed, May 20, 2009 at 10:33 AM, Barton C Massey <<a href="mailto:bart@cs.pdx.edu">bart@cs.pdx.edu</a>> wrote: <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> I don't have much to contribute, I'm afraid. :-) Just big thank-yous to Jeetu and Julien; I'm really pleased to see this move forward. Bart In message <<a href="mailto:20090518084622.GA30591@abydos.adm.naquadah.org">20090518084622.GA30591@abydos.adm.naquadah.org</a>> you wrote: > > --===============0102780725== > Content-Type: multipart/signed; micalg=pgp-sha1; > protocol="application/pgp-signature"; boundary="3MwIy2ne0vdjdPXF" > Content-Disposition: inline > > > --3MwIy2ne0vdjdPXF > Content-Type: text/plain; charset=utf-8 > Content-Disposition: inline > Content-Transfer-Encoding: quoted-printable <div class="im">> > HI, > > At 1242038193 time_t, Jeetu Golani wrote: > > I've updated my branch at github to reflect changes in code as suggested = > by > > Julien and also rectify the goof ups I made with my initial submission. </div>> >=20 <div class="im">> > Julien, I would appreciate if you could verify if all is well now. > > Yeah, that's much better. :-) > > I'll quote some of your work here and just review it. I'm not qualified > in Windows coding so I won't judge if the work is good, bad or even > functionnal. > > --- /dev/null > +++ b/src/windefs.h > [...] </div>> +#define WINVER 0x0501 /* required for getaddrinfo/freeaddrinfo defined onl= <div><div></div><div class="h5">> y for WinXP and above */ > > Hum, are you sure you always want to define it? > If not, maybe adding #ifndef WINVER / #endif surrounding this #define > might be a good idea. > > > --- a/src/xcb_in.c > +++ b/src/xcb_in.c > +static int read_block(const int fd, void *buf, const size_t len) > and > +int _xcb_in_read(xcb_connection_t *c) > > You are redefining the whole function. IMHO, this is not so good, > because you are duplicating a lot of code. And is some day we need to > fix code in one of that function, we may forget to fix it in the WIN32 > version. > > > --- a/src/xcb.h > +++ b/src/xcb.h > +int initWSA(void); > > This is not right. If I'm understanding correctly, this is required to > initialize some stuff. The function name is not respecting the naming > scheme of xcb functions and the name is too generic. > > I suggest that: > 1. You drop the function and documente that loading winsock via > WSAStartup() is mandatory before using XCB; > 2. or you add a static flag in xcb_open() that check that winsock has been > initialized before running; > 3. or you initialize winsock in xcb_open() unconditionnally. I don't know > if you can initialize winsock multiple times. If not, the solution > above can be better. But again, maybe another lib can initiliaze > winsock in its corner, so maybe if it can be initialized only once, > solution 1 will be better. > > The rest seems fine to me. > > Cheers, </div></div>> --=20 > Julien Danjou > // =E1=90=B0 <<a href="mailto:julien@danjou.info">julien@danjou.info</a>> <a href="http://julien.danjou.info" target="_blank">http://julien.danjou.info</a> <div class="im">> // 9A0D 5FD9 EB42 22F6 8974 C95C A462 B51E C2FE E5CD > // When I get sad, I stop being sad and be awesome instead. True story. > </div>> --3MwIy2ne0vdjdPXF > Content-Type: application/pgp-signature; name="signature.asc" > Content-Description: Digital signature > Content-Disposition: inline <div class="im">> > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.9 (GNU/Linux) > </div>> iEYEARECAAYFAkoRIF4ACgkQpGK1HsL+5c0fxACfZiIGTDyDnbPYUt7I02Szg5xv <div class="im">> tJsAoNFkKUc1OjxugdpAG9U/sUifmgs6 > =HMAC > -----END PGP SIGNATURE----- > </div>> --3MwIy2ne0vdjdPXF-- > > --===============0102780725== > Content-Type: text/plain; charset="us-ascii" > MIME-Version: 1.0 > Content-Transfer-Encoding: 7bit > Content-Disposition: inline <div class="im">> > _______________________________________________ > Xcb mailing list > <a href="mailto:Xcb@lists.freedesktop.org">Xcb@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/xcb" target="_blank">http://lists.freedesktop.org/mailman/listinfo/xcb</a> </div>> --===============0102780725==-- </blockquote></div>