[ofw] [RFC] [PATCH 3/3] port of libibumad to windows

[Fab Tillier]

> __declspec(dllexport)
> int     umad_set_grh_net(void *umad, void *mad_addr)

<snip...>

> __declspec(dllexport)
> int umad_set_grh(void *umad, void *mad_addr)

The two functions above are identical.  Are they supposed to be?  And if so, why two?  Also note your indentation...

> __declspec(dllexport)
> int umad_recv(int portid, void *umad, int *length, int timeout_ms)
> {
>         WM_MAD          *mad = (WM_MAD *) umad;
>         um_port_t       *port;
>         HRESULT         hr;
>
>         port = &ports[portid];
>         hr = port->prov->Receive(mad, (size_t) length, &port->overlap);
>
>         if (hr == ERROR_IO_PENDING) {

Note that ERROR_IO_PENDING isn't an HRESULT.  Either E_PENDING or HRESULT_FROM_WIN32( ERROR_IO_PENDING ).

>                 if (port->pending && timeout_ms == 0) {

How does a client call to check if anything's done without actually waiting?  I would have expected 0 to be that, but it seems that 0 means infinite (I would have picked -1 for infinite.)

>                         do {
>                                 hr = WaitForSingleObject(port-
> >overlap.hEvent, 250);

WaitForSingleObject doesn't return an HRESULT.

>                         } while (hr == WAIT_TIMEOUT && port->pending);

Just wait on the event.  If the client deregisters from a different thread, the overlapped request should get cancelled which will set the event.

>                 } else {
>                         hr = WaitForSingleObject(port->overlap.hEvent,
> (DWORD) timeout_ms);
>                         if (hr == WAIT_TIMEOUT) {
>                                 return -EWOULDBLOCK;
>                         }
>                 }
>         }
>
>         if (FAILED(hr)) {
>                 return -EIO;
>         }
>
>         if (mad->Length <= (UINT32) *length) {
>                 port->pending = FALSE;
>         }
>
>         *length = mad->Length;
>         return 0;
> }
>
> __declspec(dllexport)
> int umad_poll(int portid, int timeout_ms)
> {
>         WM_MAD          mad;
>         um_port_t       *port;
>         HRESULT         hr;
>
>         port = &ports[portid];
>         hr = port->prov->Receive(&mad, sizeof mad, &port->overlap);

What happens to the received WM_MAD?  How does a subsequent umad_recv get the data?

Also, you don't handle a call to umad_poll on one thread at the same time as a call to umad_recv on another.  As it stands, you'll have two requests using the same overlapped structure.

>
>         if (hr == ERROR_IO_PENDING) {
>                 hr = WaitForSingleObject(port->overlap.hEvent, (DWORD)
> timeout_ms);
>                 if (hr == WAIT_TIMEOUT) {
>                         return -ETIMEDOUT;
>                 }
>         }
>
>         if (FAILED(hr)) {
>                 return -EIO;
>         }
>
>         port->pending = TRUE;
>         return 0;
> }