IMO, the auto-ref class needs to go.
> +class CNDMemoryRegion : public INDMemoryRegion, private CNDBase
> {
> + auto_ref<CNDAdapter> m_pAdapter;
> + WV_MEMORY_KEYS m_Keys;
> + VOID* volatile m_pOv;
> +
> +private:
> + CNDMemoryRegion(CNDAdapter& adapter);
> + ~CNDMemoryRegion(){};
> +
> public:
Does anyone have a clue what exactly the following code does by reading it?
> + static STDMETHODIMP
> + CreateInstance(CNDAdapter& adapter, VOID** ppMemoryRegion)
> + {
> + auto_ref<CNDMemoryRegion> mr(new CNDMemoryRegion(adapter));
> + if (mr.get() == NULL) {
> + return ND_NO_MEMORY;
> + }
> +
> + *ppMemoryRegion = mr.detach();
> + return ND_SUCCESS;
> + }
*IF* I'm reading it correctly, this allocates a pointer. If the allocation
succeeds, an 'automatic reference' is taken on it, although instead of just
calling Ref(), we use auto_ref<>. We then call mr.get(), which checks to see
if the mr actually exists. So, eventhough mr.get() looks like it could
dereference a NULL mr, the check is provided by some underlying class.
Finally, if mr is valid, then we "detach" it, and return whatever that value
returns.
> + static STDMETHODIMP
> + CreateInstance(CNDAdapter& adapter, VOID** ppMemoryRegion)
> + {
> + auto_ref<CNDMemoryRegion> mr(new CNDMemoryRegion(adapter));
+ return ::HeapAlloc(WinVerbs::NetworkDirect::g_hHeap, 0, size);
> + if (mr.get() == NULL) {
> + return ND_NO_MEMORY;
> + }
> +
> + *ppMemoryRegion = mr.detach();
> + return ND_SUCCESS;
> + }
Contrast the above code with what was there:
CreateInstance(CNDAdapter *pAdapter, INDMemoryRegion** ppMemoryRegion)
{
HRESULT hr;
CNDMemoryRegion *mr;
mr = new CNDMemoryRegion(pAdapter);
if (mr == NULL) {
hr = ND_NO_MEMORY;
goto err;
}
*ppMemoryRegion = mr;
return ND_SUCCESS;
err:
*ppMemoryRegion = NULL;
return hr;
}
which is way more explicit and simpler to read/understand. I see no value in
the auto_ref obscurity and find that it makes the code more difficult to
maintain, not easier.
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
