Navigation Menu

Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Kovri: remove i2pd's endian, replace with ByteStream + Boost.Endian #809

Merged
merged 15 commits into from Feb 12, 2018

Conversation

anonimal
Copy link
Collaborator

@anonimal anonimal commented Feb 8, 2018


By submitting this pull-request, I confirm the following:

  • I have read and understood the contributor guide in kovri-docs.
  • I have checked that another pull-request for this purpose does not exist.
  • I have considered and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used and that this pull-request may be closed by the will of the maintainer.
  • I give this submission freely under the BSD 3-clause license.

Resolves #772. References #140. Referencing #511 #643 among others (see the TODOs).

This is entirely kovri code.
Removes i2pd's native-endian read implementation

Note: i2pd's free function call design procured spaghetti code. This
commit does not resolve that design issue but a bytstream refactor will.
See the TODOs regarding bytestream refactor.
Removes i2pd's big/native-endian write implementation

Note: i2pd's free function call design procured spaghetti code. This
commit does not resolve that design issue but a bytstream refactor will.
See the TODOs regarding bytestream refactor.
@anonimal
Copy link
Collaborator Author

anonimal commented Feb 8, 2018

Live tests and unit-tests pass on BE and LE machines (tested on Linux/FreeBSD/Windows).

@anonimal
Copy link
Collaborator Author

anonimal commented Feb 8, 2018

OpenBSD and OS X apparently want to /usr/local/include/boost/endian/conversion.hpp:298:12: error: call to 'endian_reverse' is ambiguous. Resolve before merging.

@coneiric
Copy link
Contributor

coneiric commented Feb 8, 2018

Great changes.

From what I've found so far, it looks like the ambiguous call error may be coming from the templated Read and Write functions.

There are multiple endian_reverse overloads for uintNN_t, which gets called by boost::endian::native_to_big_inplace() on little-endian machines.

Not sure what the right solution would be.

Maybe replacing:

if (native_to_big)
  boost::endian::native_to_big_inplace(data);

with

if (native_to_big)
  boost::endian::native_to_big_inplace(std::static_cast<decltype(data)>(data));

Then perhaps doing something similar for Read(), and other places where the boost::endian::{native_to_big*,big_to_native*} is passed a templated param?

@anonimal
Copy link
Collaborator Author

anonimal commented Feb 9, 2018

Not sure what the right solution would be.

The build failures exposed pre-existing kovri type issues. I've fixed those issues and now the build is resolved (casting is not the way to resolve the issue).

@anonimal
Copy link
Collaborator Author

anonimal commented Feb 9, 2018

Note: OSX router.info looks good, as expected, but Linux is producing a bad router.info (introduced before aforementioned patch in previous comment). Hold on merging until I resolve the issue.

@anonimal
Copy link
Collaborator Author

anonimal commented Feb 9, 2018

Resolved. Also amended // TODO(anonimal): there is no spec-defined size limit, but we need 2 bytes... when writing RI options.

@anonimal anonimal merged commit 7f5f51b into monero-project:master Feb 12, 2018
anonimal added a commit that referenced this pull request Feb 12, 2018
7f5f51b Kovri: replace glic endian conversion with boost.endian (anonimal)
944264d Kovri: finish the removal of i2pd's endian impl (anonimal)
7a12f4e Kovri: implement big/native-endian bytestream write (anonimal)
dea57f7 ByteStream: implement optional endian conversion (anonimal)
b988ea7 InputByteStream: add docs to readers (anonimal)
9a30188 OutputByteStream: add write wrapper + static writer (anonimal)
ffb2fff Kovri: implement native-endian bytestream read (anonimal)
2809972 ByteStream: simplify boost address to vector impl (anonimal)
df94728 ByteStream: virtual dtors for I/O base cases + explicit ctors (anonimal)
889f3ed ByteStream: implement I/O base class + related I/O refactor (anonimal)
3d704a1 ClientDestination: implement bytestream in Data message handler (anonimal)
f0c8acd InputByteStream: add read wrapper + static reader (anonimal)
65cdc73 InputByteStream: add new getters/members (anonimal)
7900b7e ByteStream: cpplint/clang-format/style check-in (anonimal)
5d421dd ByteStream: fix copyright dates (anonimal)
@anonimal anonimal deleted the endian branch February 12, 2018 22:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Core: throw out i2p_endian/little_big_endian, reimplement with Boost.Endian
2 participants