Fat parser should fail on file with 0 architectures #449
Conversation
lib/macho/fat_file.rb
Outdated
| @@ -346,6 +348,11 @@ def populate_fat_header | |||
| # formats. | |||
| raise JavaClassFileError if fh.nfat_arch > 30 | |||
|
|
|||
| # Rationale: fat Mach-0 files with no internal slices should | |||
| # return an error unless the permissive option is set. | |||
| permissive = options.fetch(:permissive, false) | |||
There was a problem hiding this comment.
IMO we can remove the permissive check here: there's very little a user can do with an empty universal binary, so we should fail unconditionally if we have no architecture slices.
There was a problem hiding this comment.
That makes sense.
Yep, that's what I had in mind. Re: licensing: it should be sufficient to put a license notice in the top-level README, noting that we use binaries taking from the LLVM project for the test suite. |
|
Copy and pasting the entire license would be a bit much for the top level README though, wouldn't it? Their Apache license is significantly longer than the current README. How about I just drop it in the llvm folder and then link to it from the README? |
Yep, that works. |
|
Okay, I added the license notice. |
This PR is related to issue #139.
I added a new exception called ZeroArchitectureError that gets thrown every time the permissive option is not set and nfat_arch == 0.
The only question I have is how you'd like to test for this exception. I just grabbed the llvm binary that you linked to in the original post and threw it in the test/bin/llvm/ folder and used that for testing. Is that what you were thinking? Also, that probably means that we should include some sort of llvm license in that directory.