Hi Gerald,
On 5/2/10 9:16 PM, Gerald Pfeifer wrote:
IActiveScriptSite_OnStateChange is described to return S_OK upon success, so instead of ignoring its return value and unconditionally returning S_OK it strikes me that we should return its result instead.
In this case returning S_OK is fine, but there is missing test for OnStateChange return value.
Jacek
On Mon, 3 May 2010, Jacek Caban wrote:
On 5/2/10 9:16 PM, Gerald Pfeifer wrote:
IActiveScriptSite_OnStateChange is described to return S_OK upon success, so instead of ignoring its return value and unconditionally returning S_OK it strikes me that we should return its result instead.
In this case returning S_OK is fine, but there is missing test for OnStateChange return value.
Hmm, but if you look at the patch, in those cases we call OnStateChange, we actually pass on it's return value:
hres = IActiveScriptSite_OnStateChange(site, (state = ss)); return hres;
And that is "S_OK if successful" according to MSDN. Perhaps you could follow up with a patch to refine what we have now?
Gerald
On 5/7/10 11:47 PM, Gerald Pfeifer wrote:
On Mon, 3 May 2010, Jacek Caban wrote:
On 5/2/10 9:16 PM, Gerald Pfeifer wrote:
IActiveScriptSite_OnStateChange is described to return S_OK upon success, so instead of ignoring its return value and unconditionally returning S_OK it strikes me that we should return its result instead.
In this case returning S_OK is fine, but there is missing test for OnStateChange return value.
Hmm, but if you look at the patch, in those cases we call OnStateChange, we actually pass on it's return value:
hres = IActiveScriptSite_OnStateChange(site, (state = ss)); return hres;And that is "S_OK if successful" according to MSDN.
It's a test, not a real script engine, we don't have to handle error paths, but test correct behavior instead.
Perhaps you could follow up with a patch to refine what we have now?
Sure, I've sent a patch.
Jacek
-
Gerald Pfeifer -
Jacek Caban