[PATCH] D24060: [lld][ELF] Add support for -b binary

[Rui Ueyama via llvm-commits]

It reads a binary file and wrap it with ELF header/trailer. And overall it
is part of the reader rather than the writer from the point of view of the
entire linking process. So I at least want to avoid "Writer".

On Thu, Sep 8, 2016 at 1:38 PM, Michael Spencer <bigcheesegs at gmail.com>
wrote:

> On Thu, Sep 8, 2016 at 1:27 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Doesn't BinaryFile make sense? We have InputFiles to handle input files,
>> so BinaryFile sounds like it is to handle binary file inputs. Do you have
>> other suggestions?
>>
>
> SimpleELFWriter isn't an input. It's a class that generates ELF files.
>
> - Michael Spencer
>
>
>>
>> On Thu, Sep 8, 2016 at 1:16 PM, Michael Spencer <bigcheesegs at gmail.com>
>> wrote:
>>
>>> On Thu, Sep 8, 2016 at 9:19 AM, Rui Ueyama <ruiu at google.com> wrote:
>>>
>>>> ruiu added inline comments.
>>>>
>>>> ================
>>>> Comment at: ELF/CMakeLists.txt:20
>>>> @@ -19,2 +19,3 @@
>>>>    ScriptParser.cpp
>>>> +  SimpleELFWriter.cpp
>>>>    Strings.cpp
>>>> ----------------
>>>> I still don't like this file name. `SimpleELFWriter` sounds like it is
>>>> for -o binary instead of -b. Can you rename BinaryFile.cpp?
>>>>
>>>
>>> I'm fine with finding another name, but BinaryFile isn't at all what
>>> this class is.
>>>
>>> - Michael Spencer
>>>
>>>
>>>>
>>>> ================
>>>> Comment at: ELF/InputFiles.cpp:736
>>>> @@ +735,3 @@
>>>> +template <class ELFT> std::unique_ptr<InputFile>
>>>> BinaryFile::createELF() {
>>>> +  SimpleELFWriter<ELFT> ELF(ET_REL, Config->EMachine);
>>>> +  auto DataSec = ELF.addSection(".data");
>>>> ----------------
>>>> Please move these details to SimpleELFWriter.cpp.
>>>>
>>>>
>>>> https://reviews.llvm.org/D24060
>>>>
>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160908/b39f4c9b/attachment.html>