Hey Mike, thanks for the patch — keep up the good work :)

I verified the implementation works as expected on Linux (Ubuntu 12.10), but I 
notice you're not filling in the process architecture. I believe there's an 
existing comment about why it's better to determine the arch in a different 
place, so it's probably fine to omit for now, but unless someone has any 
qualms, I will file a bug to also list the process' architectures when the user 
does "platform process list"; currently that field just shows "0".

In addition to fixing the platform command bug (functionalities/platform) this 
patch also fixes the attach-by-name test cases (python_api/hello_world and 
functionalities/process_attach)!

Committed in r181904, tests re-enabled in r181905.


Cheers,
Dan

From: Michael Sartain 
<[email protected]<mailto:[email protected]>>
Date: Tuesday, 14 May, 2013 8:06 PM
To: Michael Sartain 
<[email protected]<mailto:[email protected]>>
Cc: "[email protected]<mailto:[email protected]>" 
<[email protected]<mailto:[email protected]>>
Subject: Re: [lldb-dev] PATCH for REVIEW: Implement Linux Host::FindProcesses()

On Tue, May 14, 2013 at 4:59 PM, Michael Sartain 
<[email protected]<mailto:[email protected]>> wrote:
On Tue, May 14, 2013 at 4:42 PM, Michael Sartain 
<[email protected]<mailto:[email protected]>> wrote:
I haven't been able to run the python test script mentioned in the bug report 
yet, but "platform process list" in lldb looks correct on my Ubuntu 12.04 
64-bit machine.

We got "./dotest.py -C clang -v -t -f 
PlatformCommandTestCase.test_process_list" to run and it all appears to pass 
now except this line in TestPlatformCommand.py:

    @expectedFailureLinux # due to bugzilla 14806 -- "platform status" prints 
more information on Mac OS X than on Linux
    def test_status(self):
        self.expect("platform status",
            substrs = ['Platform', 'Triple', 'OS Version', 'Kernel', 
'Hostname'])

platform status returns this on my machine:

(lldb) platform status
Linux 3.5.0-28-generic #48~precise1-Ubuntu SMP Wed Apr 24 21:42:24 UTC 2013

Not sure how this test should be modified to expect something along these lines 
and also handle all the other Linux distros? I'll leave this to someone else... 
:)

I see - this looks like it has a separate bug filed for it:

http://llvm.org/bugs/show_bug.cgi?id=14806

PlatformLinux::GetStatus() in PlatformLinux.cpp just needs a little bit of 
love. We'll take a look at that fixing that...

_______________________________________________
lldb-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev

Reply via email to