Closed Bug 1368033 Opened 3 years ago Closed 3 years ago
Dll Interceptor .exe | test failed with return code 1
1.68 KB, patch
|Details | Diff | Splinter Review|
09:46:18 INFO - TEST-FAILED | WindowsDllInterceptor | Failed to execute hooked function SetCursorPos from user32.dll Perhaps the cursor is changing between GetCursorPos and SetCursorPos? I wonder if it would make more sense to just give SetCursorPos garbage and assert that it fails.
> Perhaps the cursor is changing between GetCursorPos and SetCursorPos? > > I wonder if it would make more sense to just give SetCursorPos garbage and > assert that it fails. Actually, its the SetCursorPos call that is failing -- the test shouldn't care if the cursor changes positions between calls. Indeed, docs don't mention how SetCursorPos _can_ fail (it takes 2 ints as position and clips them if they are out of bounds so all inputs are valid). Usually, this means a security failure. Rather than care what the answer is, I've removed the return value check. The tests are really intended to be a sanity check that we didn't write garbage assembler into the harness. Their failures are almost certainly going to be crashes. (You'll see other methods in there where I don't check the return value for because they were also problematic.)
Attachment #8872779 - Flags: review?(dmajor)
Assignee: nobody → davidp99
Comment on attachment 8872779 [details] [diff] [review] ignoreReturn.patch Oh, if all inputs are valid then perhaps we should just pass 512,512 all the time?
Attachment #8872779 - Flags: review?(dmajor) → review+
I've removed the GetCursorPos call and just used 512,512 as you suggested. I was resisting that as we've now gone full circle back to my _original_ implementation in the old bug... but the even mild additional complexity wasn't actually warranted.
Attachment #8872779 - Attachment is obsolete: true
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/a12d99ba297f Ignore return value of SetCursorPos in TestDllInterceptor. r=dmajor
You need to log in before you can comment on or make changes to this bug.