[Xcb] xcb_in.c read_block() error handling looks wrong for _WIN32

[Uli Schlachter]

Hi,

Am 11.08.2015 um 18:52 schrieb Jeremy Huddleston Sequoia:
> In read_block in src/xcb_in.c, we see:
> 
>         if(...) {
>             ...
>             /* Initializing errno here makes sure that for Win32 this loop will execute only once */
>             errno = 0;
>             do {
>                 ret = select(fd + 1, &fds, 0, 0, 0);
>             } while (ret == -1 && errno == EINTR);
> #endif /* USE_POLL */
>         }
>         if(ret <= 0)
>             return ret;
> 
> 
> I'm not much of an expert of _WIN32, but based on the comment, this looks wrong.  From the comment, am I assume that on _WIN32, select(2) does not set errno or maybe doesn't set it to EINTR in the case the analogous EINTR case on _WIN32.  However, the line immediately following will exit the function for _WIN32.
> 
> I don't know what the right solution is because I'm making a lot of guesses about Win32, but it just looks wrong to me, so someone more aware of Win32 should maybe take a look at this.

Sorry, but I don't really understand the problem.

So yeah, from the comment I also assume that select() does not set errno. So on
Windows there are three cases:

(a) select() succeeds, but nothing can be read (ret=0): The function returns 0.
(b) select() succeeds and our FD is readable (ret=1): The function continues.
(c) select() fails (ret=-1): The function returns -1.

On Unix, the same cases exists. There is one additional possibility:

(d) select() fails, because it is interrupted by a signal. The signal handler
will be called and select() sets errno to EINTR. After the signal handler
finished, the code here continues and will call select() again.

Which of these cases seem wrong to you?

For context: This code is in the function read_block() which is called for
reading <len> bytes from the socket. The only caller is _xcb_in_read_block() and
if read_block() returns <= 0, this calls _xcb_conn_shutdown() and treats it as
an error. So case (a) above is treated as an error, but because we do not
specify a timeout to select, it should always wait until a FD is readable, and
so (a) cannot really occur.

Cheers,
Uli
-- 
"Every once in a while, declare peace. It confuses the hell out of your enemies"
 - 79th Rule of Acquisition