search

aclements / libelfin

C++11 ELF/DWARF parser
MIT License
314 stars 99 forks source link

CMake building and Initial REL(A) support #37

Open deadly-platypus opened 5 years ago

deadly-platypus commented 5 years ago

I needed to at least be able to get relocation entries for my work, so I quickly added that by adding two new methods of the section class, get_rels and get_relas. Both return a std::vector of a new struct Elf_Rel or struct Elf_Rela instances. Additionally, I added CMake support, including the creation of the to_string.cc files in both the dwarf and elf directories. This PR only builds the libraries in those directories, with targets for the shared and static libraries. So the tests or examples still need work.

Now comes the bad part. I had my IDE reformat all the code, which created a large diff. You can take it as is, or let me know what styling you use, and I can update this PR. Or, you know, you could just say no to it all.

nicolas17 commented 4 years ago

I think CMake, reformatting code, and adding rels should be separate pull requests.

deadly-platypus commented 4 years ago

Any suggestion as to how to do that?

aclements commented 4 years ago

Hi @deadly-platypus . Unfortunately, with all of the code reformatting I can't really review or submit this. Please just follow the style of the existing code, which should be pretty consistent (though I didn't run a formatting tool, so I'm sure there are minor inconsistencies).

I'm curious about the CMake support you added, since this is the second PR to propose that. What's the benefit of adding CMake support? Make is certainly not the best thing in the world, but the existing makefiles work and are pretty simple. (I've never used CMake myself, so perhaps the answer is obvious to someone more familiar with it.)

deadly-platypus commented 4 years ago

I totally understand rejecting this PR for the formatting. I'll try to fix that soon.

Cmake is used to generate a build system, be it make files, Visual Studio... Whatever they use...., Ninja files, or others. It thus makes it easy to integrate other repos into existing projects as git submodules. So to add this repo as a dependency for my project, I as it as a submodule, and then add two lines to the CMakeLists.txt file. One line is to tell CMake about the directory, and one line to add the build target as a dependency. CMake then ensures that libelfin is built before my code, and I don't need to know how your code is built because CMake will generate the correct build system for everything. IMO it's a pretty slick system for better cross platform integration and usability.

madebr commented 3 years ago

@aclements @deadly-platypus Are you (=the authors) open to accepting a cmake pull request? I'm asking because atm, there is a request at conan-center-index for adding a libelfin recipe which includes a cmake script.

If you're open to adding a cmake script upstream, then we don't have to maintain one ourselves. (you can always tag/ask us if you happen to have problems/questions once merged)