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

[Michael Spencer via llvm-commits]

On Thu, Sep 8, 2016 at 1:47 PM, Rui Ueyama <ruiu at google.com> wrote:

> 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".
>

SimpleELFWriter doesn't read anything. It writes ELF files. If we want to
avoid writer I'm fine with SimpleELFCreator.

- Michael Spencer


>
> 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/5bc1e822/attachment-0001.html>