mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevin Klues <klue...@gmail.com>
Subject Re: Review Request 49559: Added ELFIO as bundled dependency in Mesos.
Date Sat, 02 Jul 2016 22:34:15 GMT


> On July 2, 2016, 9:51 p.m., Benjamin Mahler wrote:
> > 3rdparty/Makefile.am, line 175
> > <https://reviews.apache.org/r/49559/diff/2/?file=1435736#file1435736line175>
> >
> >     Looks like we need the following:
> >     
> >     ```
> >     $(ELFIO)/elfio/elf_types.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_dump.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_dynamic.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_header.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_note.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_relocation.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_section.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_segment.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_strings.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_symbols.hpp: $(ELFIO)-stamp
> >     $(ELFIO)/elfio/elfio_utils.hpp: $(ELFIO)-stamp
> >     ```

Strange. It built fine for me. Then again, I run with `make -j`, so maybe the stamp got created
from another target for me concurrently, so I didn't notice. At one point I was using the
target:

`$(ELFIO)/elfio/%.hpp`

But it didn't like that for some reason.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49559/#review140510
-----------------------------------------------------------


On July 2, 2016, 8:08 p.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49559/
> -----------------------------------------------------------
> 
> (Updated July 2, 2016, 8:08 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5767
>     https://issues.apache.org/jira/browse/MESOS-5767
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This includes a patch file to ELFIO to fix 2 off-by-one errors in the
> parsing of NOTE sections. Patches for these bugs have been submitted
> upstream. Details of the patches are below:
> 
> 1) Fixed off-by-one error in 'name' of add_note() function.
> 
> Previously, when assigning 'name' as a string, it's length was
> specified using the full length of 'namesz'. However, this length
> includes the trailing '\0' of the underlying char[]. This ultimately
> causes the C++ string that is created to (incorrectly) contain the
> '\0' character as well. This leads to problems where e.g. the
> following will return false, even when 'name' itself contains the
> string "GNU\0":
> 
>   if (name == "GNU") {
>     return true;
>   }
>   return false;
> 
> To fix this, we should only include the length of the string minus the
> trailing '\0'.
> 
> 2) Fixed alignment of 'desc' in add_note() function.
> 
> The ELF spec specifically lists the alignment of the namez char[] to
> be 4 bytes. To quote it:
> 
> "Padding is present, if necessary, to ensure 4-byte alignment for the
> descriptor. Such padding is not included in namesz."
> 
> However, the current implementation sets the alignment to either 4 or
> 8 bytes depending on the class of the ELF file (CLASS32 or CLASS64).
> This commit fixes the alignment to only 4 bytes in all cases.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 15f3171 
>   3rdparty/Makefile.am bd990cc 
>   3rdparty/cmake/Versions.cmake 7b73f8f 
>   3rdparty/elfio-3.1.patch PRE-CREATION 
>   3rdparty/elfio-3.1.tar.gz PRE-CREATION 
>   3rdparty/versions.am 203656c 
>   LICENSE eb39f6d 
>   configure.ac 321436b 
>   src/Makefile.am 52d63f2 
>   support/coverage.sh ab9564b 
> 
> Diff: https://reviews.apache.org/r/49559/diff/
> 
> 
> Testing
> -------
> 
> Made sure `elfio-3.1` appears in `/build` after running `../configure`, `make`.
> Made sure `elfio` appears in `/include` of installation folder after `make install`.
> 
> This second one is necessary because `stout` relies on elfio in order to function (similar
to how we've done with `picojson.h` in the past).
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message