portable serializer: use signed char for size #1531

Merged
merged 1 commit into from Jan 9, 2017

Projects

None yet

3 participants

@kenshi84
Contributor
kenshi84 commented Jan 5, 2017

See comment in PR #1515

I believe this fix doesn't have any real effect on monero with arm7 & armv8 builds, as there is no data to be serialized in Monero that is negative.

@@ -37,7 +37,8 @@ enum portable_binary_archive_flags {
//#endif
inline void
-reverse_bytes(char size, char *address){
+reverse_bytes(signed char size, char *address){
+ BOOST_ASSERT(size > 0);
@moneromooo-monero
moneromooo-monero Jan 6, 2017 Contributor

I'd use >=, maybe. reverse_bytes sounds like something only used for endianness, so may not matter in practice.
Also an throw is probably best, since it doesn't end the program, but fails the serialization.

@kenshi84
kenshi84 Jan 6, 2017 Contributor

But in both of iarchive and oarchive, the case where size==0 is clearly excluded. Should I still use >= ?

@moneromooo-monero
moneromooo-monero Jan 6, 2017 Contributor

Nah, that's fine.

@moneromooo-monero
moneromooo-monero Jan 6, 2017 edited Contributor

I still think an exception is best here though. Unless the calling code is really trivially provable to never send 0 ? In which case it'd be fine too. I've not looked.

@kenshi84
kenshi84 Jan 7, 2017 Contributor

Replaced assertion with exception. I'm pretty sure that the calling code never sends size<=0, though.

@kenshi84 kenshi84 portable serializer: use signed char for size
9d1d3a4
@fluffypony

Reviewed

@fluffypony fluffypony merged commit 9d1d3a4 into monero-project:master Jan 9, 2017

7 of 10 checks passed

buildbot/monero-static-debian-armv8 Build done.
Details
buildbot/monero-static-ubuntu-arm7 Build done.
Details
buildbot/monero-static-win32 Build done.
Details
buildbot/monero-static-freebsd64 Build done.
Details
buildbot/monero-static-osx-10.10 Build done.
Details
buildbot/monero-static-osx-10.11 Build done.
Details
buildbot/monero-static-osx-10.12 Build done.
Details
buildbot/monero-static-ubuntu-amd64 Build done.
Details
buildbot/monero-static-ubuntu-i686 Build done.
Details
buildbot/monero-static-win64 Build done.
Details
@kenshi84 kenshi84 deleted the kenshi84:portable-archive-char branch Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment