Thanks for the revised patch, Samuel. I tested and committed in r186114, which is looking green on the buildbots,
- Ashok From: [email protected] [mailto:[email protected]] On Behalf Of Thirumurthi, Ashok Sent: Wednesday, July 10, 2013 5:16 PM To: Samuel Jacob; [email protected] Subject: Re: [lldb-dev] Patch to add API to read ELF ProgramHeader/Segment Welcome back, Samuel, and thanks for the patch. A few minor thoughts: Consider testing id first so that parsing isn't required on the failure path. Also, prefer nullptr over NULL throughout: + if (!ParseProgramHeaders() || !id) + return NULL; Consider adding a failure patch if GetProgramHeaderByIndex returns nullptr: + const elf::ELFProgramHeader *segment_header = GetProgramHeaderByIndex(id); + return DataExtractor(m_data, segment_header->p_offset, segment_header->p_filesz); Maybe avoid the cross-post also so we have just one thread for the review. Cheers, - Ashok From: [email protected]<mailto:[email protected]> [mailto:[email protected]] On Behalf Of Samuel Jacob Sent: Wednesday, July 10, 2013 3:03 PM To: lldb-dev; [email protected]<mailto:[email protected]> Subject: [lldb-dev] Patch to add API to read ELF ProgramHeader/Segment The attached patch adds 3 new functions to ObjectFileELF which enables accessing ELF segment data possible. Segment data is needed to support elf core files. Please review and commit(if ok). Thanks Samuel
_______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
