[Xcb] Win32 branch of XCB now up :)

[Barton C Massey]

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 <20090518084622.GA30591 at abydos.adm.naquadah.org> 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
> 
> 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.
> >=20
> > 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
> [...]
> +#define WINVER 0x0501 /* required for getaddrinfo/freeaddrinfo defined onl=
> 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,
> --=20
> Julien Danjou
> // =E1=90=B0 <julien at danjou.info>   http://julien.danjou.info
> // 9A0D 5FD9 EB42 22F6 8974  C95C A462 B51E C2FE E5CD
> // When I get sad, I stop being sad and be awesome instead. True story.
> 
> --3MwIy2ne0vdjdPXF
> Content-Type: application/pgp-signature; name="signature.asc"
> Content-Description: Digital signature
> Content-Disposition: inline
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> 
> iEYEARECAAYFAkoRIF4ACgkQpGK1HsL+5c0fxACfZiIGTDyDnbPYUt7I02Szg5xv
> tJsAoNFkKUc1OjxugdpAG9U/sUifmgs6
> =HMAC
> -----END PGP SIGNATURE-----
> 
> --3MwIy2ne0vdjdPXF--
> 
> --===============0102780725==
> Content-Type: text/plain; charset="us-ascii"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline
> 
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/xcb
> --===============0102780725==--