[llvm] r256156 - [llvm-objdump] Use appropriate helper. NFC.

[Davide Italiano via llvm-commits]

On Tue, Jan 12, 2016 at 2:54 PM, Davide Italiano <davide at freebsd.org> wrote:
> On Tue, Jan 12, 2016 at 2:24 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>> On Tue, Jan 12, 2016 at 11:57 AM, Davide Italiano <davide at freebsd.org>
>> wrote:
>>>
>>> On Tue, Jan 12, 2016 at 11:45 AM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>> >
>>> >
>>> > On Mon, Dec 21, 2015 at 6:10 AM, Davide Italiano via llvm-commits
>>> > <llvm-commits at lists.llvm.org> wrote:
>>> >>
>>> >> Author: davide
>>> >> Date: Mon Dec 21 08:10:54 2015
>>> >> New Revision: 256156
>>> >>
>>> >> URL: http://llvm.org/viewvc/llvm-project?rev=256156&view=rev
>>> >> Log:
>>> >> [llvm-objdump] Use appropriate helper. NFC.
>>> >
>>> >
>>> > Test case?
>>> >
>>>
>>> test/tools/llvm-objdump/invalid-input.test should somehow cover this,
>>> I guess.
>>
>>
>> But there were no changes for that test when this commit was made - so it
>> doesn't seem like it would be testing the change you made (in the sense that
>> it wouldn't catch a regression caused by reverting this patch).
>>
>> What would fail if I revert this patch or otherwise broke the functionality
>> you were fixing in this commit?
>>
>
> hmm, I understand you concerns, sorry if it came out as I didn't.
> I'll try to build a broken MachO executable and check that in.
>
>

OK, it seems code is simply dead (and therefore could be removed).

In particular:
* If -macho is specified, we already bail out correctly with invalid
object file much earlier:

(gdb)
1480      if (std::error_code EC = BinaryOrErr.getError()) {
(gdb)
1481        errs() << "llvm-objdump: '" << Filename << "': " <<
EC.message() << ".\n";
(gdb)
llvm-objdump: 'hello-macho.bin.2': The file was not recognized as a
valid object file.

* If the object is valid (but not MachO), that's covered by the if
statement above the line I changed in the commit.

Both of these are, alas, untested.
I propose to:
1) Add test for these
2) Remove the whole else {} condition I previously changed.

Also, CC:ing Kevin Enderby as he knows it much better than I do.
Kevin, what do you think?

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare