Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-portable type used in wire protocol definition #355

Open
iamsmooth opened this issue Jul 27, 2015 · 3 comments · May be fixed by #9659
Open

Non-portable type used in wire protocol definition #355

iamsmooth opened this issue Jul 27, 2015 · 3 comments · May be fixed by #9659

Comments

@iamsmooth
Copy link
Contributor

Similar to issue #88, connection_entry in p2p_protocol_defs.h contains is_income (misspelling of is_incoming I believe) which is defined as a bool. The size of bool is implementation-defined.

Fortunately connection_entry is only used for node debugging, which we may not even have enabled. Nevertheless the code should be fixed if it isn't going to be removed.

@moneromooo-monero
Copy link
Collaborator

I think this is invalid, as the struct is not written as is, but the fields are serialized independently, and bool has an overload in the serialization code.

@iamsmooth
Copy link
Contributor Author

Quite possible. Where is this overload?

@moneromooo-monero
Copy link
Collaborator

It's in the list of handled types in:
struct base_serializable_types: public boost::mpl::vector<uint64_t, uint32_t, uint16_t, uint8_t, int64_t, int32_t, int16_t, int8_t, double, bool, std::string, typename t_storage::meta_entry>::type
but after that, I can't follow the serialize template code to know exactly where it does. I suspect serialize_int, but I'm not sure.

lxop pushed a commit to lxop/monero that referenced this issue Jan 27, 2021
@jeffro256 jeffro256 linked a pull request Dec 27, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants