On Tue, 2 Mar 2004, Gisle Vanem wrote:

> "David Byron" said:
>
> Looks oaky at first sight, except:
>
> @@ -1079,7 +1175,8 @@
> objs[1] = event_handle;
>
> /* Tell winsock what events we want to listen to */
> - if(WSAEventSelect(sockfd, event_handle, FD_READ|FD_CLOSE)
> == SOCKET_ERROR) {
> + if(event_select_func(sockfd, event_handle,
> FD_READ|FD_CLOSE) == SOCKET_ERROR) {
> + FreeLibrary(wsock2);
> return 0;
> }
>
> Shouldn't the event-handle be closed here and some other places below?
> Looks like WSACloseEvent() was omitted by mistake(?) in the
> original code.

I agree. My goal wasn't really to fix bugs in the original code, but why
not? I've attached a patch that calls (a dynamically loaded) WSACloseEvent
in what I think are the right places, and is generally a bit more anal about
cleaning things up.

> And calling a func-ptr like
> event_handle = (WSAEVENT)create_event_func();
> should IMHO be written as:
> event_handle = (WSAEVENT) (*create_event_func)();
>
> This is misused in other places in libcurl (ldap.c etc.). I
> don't know if some compilers have problems with the syntax,
> but it's valuable to see it's an indirect call.

My habit is to write the calls the first way -- the one you don't like. I
think you're probably right that (*create_event_func) is more correct, but
I'd have to do some digging to convince myself. Do you mind leaving them as
they are for now and looking into this in another patch?
 
> BTW. I think MSVC should have HAVE_LONGLONG defined in it's Makefile
> to support large files.

I'd love to deal with this later if that's OK.

Thanks for the feedback.

-DB