naphaso / cbor-cpp

CBOR C++ serialization library
75 stars 29 forks source link

decoder-neg-int-fix #10

Closed mjokost closed 6 years ago

mjokost commented 7 years ago

Negative ints are encoded as positive ints but with -1 since 0 is not in the set, so they should be adjusted by one when decoding.

cstarkj commented 7 years ago

This was definitely an error I found. However your fix is incorrect. The data is tagged as a negative number but is encoded as a positive number -1. for example -360 is sent as a +359. hence when you decode this you need to add 1 to the encoded positive number and then negate it.

_listener->on_integer(-(int) (_in->get_byte() + 1) );

mjokost commented 7 years ago

Thanks cstartj, but I am a bit unclear on what you want me to change. The fix and your suggestion seem to do the same thing:

// -(int) 359 - 1 = -359 - 1 = -360 _listener->on_integer(-(int) _in->get_byte() - 1); // -(int) (359 + 1) = -360 _listener->on_integer(-(int) (_in->get_byte() + 1) );

Is there an advantage of doing it in the latter form?

cstarkj commented 7 years ago

My apologies, I guess I didn't look closely enough at the Pull Request. Either way fixes the issue. I don't think one is better than the other.

When you merge this PR I have some enhancements I would like to push up to you.

  1. Added a new decoder (copied/modified the decoder::run method) which allows the caller to repeatably call it and obtain the values of each encoded entry in the stream.
  2. Added a new method to the output class that will reset the output buffer to zero so the object instance can be reused to encode additional cbor streams Thanks