tfiala added inline comments.

================
Comment at: test/test_runner/lib/process_control.py:214
@@ +213,3 @@
+        """
+        raise Exception("platform needs to implement")
+
----------------
zturner wrote:
> Should this return `False` now that this is not supported on Windows, or do 
> you think this is still ok?  It's probably just of theoretical concern.
Correct - this is not valid if soft terminate isn't supported.  It should 
return false if soft terminate is not supported, and raise if it is supported 
but not overriddden, I think.

================
Comment at: test/test_runner/lib/process_control.py:229
@@ +228,3 @@
+        """
+        raise Exception("platform needs to implement")
+
----------------
zturner wrote:
> Change this to:
> 
>     # It's not actually documented what return code Popen.terminate()
>     # passes to TerminateProcess.  Experimentation shows that it's always
>     # 1, but there's no guarantee that's true.  Hopefully this check
>     # works well enough to be sufficient.
>     return returncode != 0
> 
> With these changes I get the following output:
> 
>     d:\src\llvm\tools\lldb\test\test_runner\test>python 
> process_control_tests.py
>     ....ss
>     ----------------------------------------------------------------------
>     Ran 6 tests in 1.208s
> 
>     OK (skipped=2)
> 
> Does that look right to you?
> It's not actually documented what return code Popen.terminate()
passes to TerminateProcess. Experimentation shows that it's always

This is checking the returncode as returned by wait() on the process.  So we're 
checking the return value of popen_object.wait() here (which should be 
identical to the popen_object.returncode() immediately after wait() succeeds).

I think for the Windows case, we override this in the WindowsProcessHelper and 
have that return "True".  (All kills will be hard terminate in this case).  
This is really only used by the test cases right now.  Alternatively, we can 
change it to pass in the popen object, and have us write in a "did a hard kill 
on this" member variable on the Windows implementation.  Then, we just return 
"True" if that is set, False otherwise.

For Windows, though, I can put in the returncode != 0.  If the hard kill test 
works with that, it is fine.  But it will also pass if a process returns a 
non-zero exit code I think (i.e. return from main() is non-zero).


http://reviews.llvm.org/D13124



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to