[llvm] r221915 - Object, Mach-O: Refactor and clean code up

Kevin Enderby enderby at apple.com
Fri Jan 23 14:14:02 PST 2015


Added Lang to the To: line and he maybe better to comment.

While the change looks cleaner I’m not sure what the error handling should be for malformed Mach-O files in llvm’s libObject.  If malformed Mach-O files cause asserts it is difficult to build tools to print and diagnose bad files.  For example in Mac OS otool(1) is used both to display information about good Mach-O files and do its best to display what it can with warning about  malformed Mach-O files.  For the guy producing Mach-O files which some times are broken this is a valuable approach for have otool(1) be able so show what it can instead of just stopping.

My thoughts,
Kev

> On Jan 23, 2015, at 1:49 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> 
> ping.
> 
> Felipe's work has improved things in here, but the attached patch still passes all tests.
> 
> 
> On 10 December 2014 at 14:35, Rafael Espíndola <rafael.espindola at gmail.com <mailto:rafael.espindola at gmail.com>> wrote:
> On 18 November 2014 at 21:04, Rafael Espíndola
> <rafael.espindola at gmail.com <mailto:rafael.espindola at gmail.com>> wrote:
> >> -      assert(!DataInCodeLoadCmd && "Multiple data in code tables");
> >> +      // Multiple data in code tables
> >> +      if (DataInCodeLoadCmd) {
> >> +        EC = object_error::parse_failed;
> >> +        return;
> >> +      }
> >
> > Do you have a testcase for this? As with COFF, an assert/unreachable
> > is better unless we have a test showing how this is actually reached.
> 
> Ping.
> 
> I can revert this patch (revert attached) and check-all still passes.
> 
> Cheers,
> Rafael
> 
> <t.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150123/0f9de5b0/attachment.html>


More information about the llvm-commits mailing list