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

[Sean Hefty]

>> __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...

See Hal's response

The indentation is a result of basing the file off the linux version, and not
seeing that tabs were used instead of spaces (because of 4 character tabs).
I've tried to update everything to use spaces.

>> __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 ).

The returned value is HRESULT_FROM_WIN32(GetLastError()), so I changed this to
E_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.)

timeout_ms = 0 does this.  It only blocks if a MAD is available.  The desired
behavior is to return a MAD if timeout_ms = 0 after umad_poll returns 0.

>>                         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.

Deregister won't cancel outstanding overlapped operations.  Reads are on the
provider, not a specific registration.

>>                 } 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?

Note that WM_MAD does not provide space for the actual 256+ byte MAD.  Since the
buffer is too small to receive the actual MAD data, the WM_MAD header will be
returned on the next read.

Actually, the returned status from the kernel is STATUS_MORE_ENTRIES, so I need
to figure out what HRESULT that is and update this.

>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.

I asked about multi-threaded use on the general mail list, and existing apps
don't do this.  I don't think there's any clean way for the interface to
interpret what this would mean anyway. 

- Sean