Page MenuHomePhabricator

Fix TestStubSetSID after unification in to a single lldb-server binary.

Authored by sivachandra on Feb 20 2015, 2:37 PM.



lldb-server needs an explicit 'g' or 'p' argument now. Since lldb-server
is started as a gdbserver in this test, 'g' should be passed to it

Diff Detail


Event Timeline

sivachandra retitled this revision from to Fix TestStubSetSID after unification in to a single lldb-server binary..
sivachandra edited the test plan for this revision. (Show Details)
sivachandra added reviewers: flackr, ovyalov.
sivachandra updated this object.
sivachandra added a subscriber: Unknown Object (MLST).
ovyalov accepted this revision.Feb 20 2015, 2:48 PM
ovyalov edited edge metadata.
ovyalov added inline comments.
18 ↗(On Diff #20438)

Since lldb-server is supposed to be used on all platform except OSX you may use such alternative condition:

return [] if 'darwin' in sys.platform else ['g']
This revision is now accepted and ready to land.Feb 20 2015, 2:48 PM
sivachandra edited edge metadata.

Address ovyalov's comment.

This revision was automatically updated to reflect the committed changes.
vharron added inline comments.
since there's only 0-1 elements in the array, why bother with the join?  Why not just 

# returns "g" to make lldb-server run in gdbserver mode
def get_debugserver_mode():
    return "" if 'darwin' in sys.platform else "g"

stub_sid = self.get_stub_sid(' %{mode} --setsid'.format(mode=get_debugserver_mode()))
sivachandra added inline comments.Feb 20 2015, 3:42 PM

Why lists instead of strings?

It is safer to use lists for command args. See for example

"... it is recommended to pass args as a sequence."

stub_sid = self.get_stub_sid(' %{mode} --setsid'.format(mode=get_debugserver_mode()))

I do not know if |mode| is relevant terminology in case of debugserver. But if you think it holds for debugserver as well, and that you want this changed, I will do it.

I'm not particular in this case. If you agree, change it. If not, please leave it as is.