[lldb-dev] PATCH for REVIEW: Implement Linux Host::FindProcesses()

[Kaylor, Andrew]

FWIW, I was the one who put the comment in about getting the architecture elsewhere.  I put it there when I implemented GetProcessInfo for the attach by pid case.  In that situation, if I left it blank it was getting set later in the attach flow when ProcessPOSIX added the executable module to its module list.  Since the executable image was already being parsed there, I didn't want to parse it again just before we got there.

I was also really hoping to avoid adding a third (or possibly fourth) bit of code to the LLVM/LLDB code base to read ELF headers, and I didn't have time to figure out a good way to get at one of the other bits of code that was doing so.

My point in all of this is to say, yes, go ahead and get the architecture there without worrying about what that comment meant.

-Andy

-----Original Message-----
From: lldb-dev-bounces at cs.uiuc.edu [mailto:lldb-dev-bounces at cs.uiuc.edu] On Behalf Of Malea, Daniel
Sent: Wednesday, May 15, 2013 11:40 AM
To: Michael Sartain
Cc: lldb-dev at cs.uiuc.edu
Subject: Re: [lldb-dev] PATCH for REVIEW: Implement Linux Host::FindProcesses()

Great! If you're already working on figuring out the process architecture, I won't bother filing a bug about it.

Cheers,
Dan


From: Michael Sartain <mikesart at valvesoftware.com<mailto:mikesart at valvesoftware.com>>
Date: Wednesday, 15 May, 2013 2:10 PM
To: Daniel Malea <daniel.malea at intel.com<mailto:daniel.malea at intel.com>>
Cc: Michael Sartain <mikesart at valvesoftware.com<mailto:mikesart at valvesoftware.com>>, "lldb-dev at cs.uiuc.edu<mailto:lldb-dev at cs.uiuc.edu>" <lldb-dev at cs.uiuc.edu<mailto:lldb-dev at cs.uiuc.edu>>
Subject: Re: [lldb-dev] PATCH for REVIEW: Implement Linux Host::FindProcesses()

On Wed, May 15, 2013 at 10:57 AM, Malea, Daniel <daniel.malea at intel.com<mailto:daniel.malea at intel.com>> wrote:
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".

Ha! You are good. I'm actually about halfway through a GetELFProcessCPUType() function right now. It looks like I have to follow the exe link and read the elf header to get that data, but it's not too bad. Chris Lattner was kinda enough to get me commit access, so hopefully I'll have something for folks to review later today and then try to submit it myself if it looks ok.

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.

Thank you much for checking and getting this in Daniel. Appreciate it.
 -Mike

_______________________________________________
lldb-dev mailing list
lldb-dev at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev