conditional assignment syntax

[Ebobtron]

Hello,

In the following examples I assign a value to hr, then test it, then display it.
hr = ps->GetFormat(&pamt);
if(FAILED(hr))
{
showHR(hw,hr,"Get Selected Format ");
return NULL;
}

Or if you perfer

hr = ps->GetFormat(&pamt);
if(FAILED(hr)){
showHR(hw,hr,"Get Selected Format ");
return NULL;
}
Is there anything wrong with this usage,
if(FAILED(hr = ps->GetFormat(&pamt)))
{ showHR(hw,hr,"Get Selected Format "); return NULL; }it seems to work? :)

Thanks :thanks:
Rob

[Kopernikus]

Depends on FAILED and GetFormat.

For example, if FAILED is a macro that uses its parameter more than once and GetFormat returns different values than there could be problems.

Like for example here:

#definde ABS(x) ((x) > 0 ? (x) : -(x))

ABS(random())

[foxyshadis]

http://msdn2.microsoft.com/en-us/library/ms693474.aspx is the definition, a simple test for the high bit set, so it's safe to call anything in it. On the other hand, any APIs that can return a non-negative number on failure or warning condition (such as S_FALSE) need to be checked for as well.

[Ebobtron]

@foxyThanks I understand, I think, or I should say that I get what FAILED does.

It is not common to see assignments within argument lists.

(hr = the return value) as compared to

hr = the functions return value
(hr)

I was wondering, if I never see it, there is maybe a real good reason.
I am trying to get the error handling to fade into the background so I can focus on the interface.

My statements check for the error before displaying it. But I wish to tell the display function where in the program I am. Maybe I need to rewrite the showHR to something like this

bool myjunk::hrErr(HWND h,char*title,HRESULT hr)
{
/// this is almost text book
if(FAILED(hr))
{
TCHAR aa[MAX_ERROR_TEXT_LEN]; aa[0] = '\0';
if(!AMGetErrorText(hr,aa,MAX_ERROR_TEXT_LEN))
StringCbPrintf(aa,sizeof aa,"Unknown Error: 0x%X",hr);
MessageBox(h,aa,title,0);
return true;
}
return false;
}
then maybe I could do


if(hrErr(hdlg,"Function: Get Format failed",ps->GetFormat(&pamt)))
{
return NULL; // or not return, failure is not always terminal
// maybe the pins are not connected on this pass
}it's prettier too :)

[squid_80]

Virtualdub uses something very similar in it's directshow code:
#define DS_VERIFY(exp, msg) if (FAILED(hr = (exp))) { VDDEBUG("Failed: " msg " [%08lx : %s]\n", hr, GetDXErrorName(hr)); } else

This allows for clean statements like this:DS_VERIFY(mpGraphBuilder->Connect(mpRealCapturePin, pCapSplitIn), "connect capture -> dv splitter");

[Ebobtron]

@ squid_80

Thank you