You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kim van der Riet <ki...@redhat.com> on 2016/03/17 18:34:04 UTC

Review Request 44967: Add operator<< to proton::byte_array (and hence to decimal32, decimal64 and decimal128)

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

Review request for qpid and Alan Conway.


Bugs: PROTON-1159
    https://issues.apache.org/jira/browse/PROTON-1159


Repository: qpid-proton-git


Description
-------

proton::byte_array currently has no method to stream values. To help with qpid-interop-test, a basic version of operator<<() which returns a hex representation of the byte array in the format "0xNNNNNN" of the appropriate length is suggested. The decimal classes use "decimalXX(0xNNNN)" as their format.

An bug in proton::uuid::operator<<() is also presented in which bytes > 0x80 within the UUID sequence (negative char values) print as ffffffnn rather than nn.


Diffs
-----

  proton-c/bindings/cpp/include/proton/byte_array.hpp fe3f864 
  proton-c/bindings/cpp/src/decimal.cpp 93fe84c 
  proton-c/bindings/cpp/src/uuid.cpp f224df7 

Diff: https://reviews.apache.org/r/44967/diff/


Testing
-------

Works as expected in qpid-interop-test.


Thanks,

Kim van der Riet


Re: Review Request 44967: Add operator<< to proton::byte_array (and hence to decimal32, decimal64 and decimal128)

Posted by Alan Conway <ac...@redhat.com>.

> On March 17, 2016, 7:28 p.m., Alan Conway wrote:
> > This overlaps a bit with some changes I made in my last commit. I rebased onto the latest master and made minor changes:
> > - hide implementation of ostream<< op in .cpp file
> > - consistent ostream ops in uuid, binary and byte_array: 
> >   - all use of ios_guard to restore state flags
> >   - all use new printable_byte() function to convert a signed/unsigned char value to an unsigned, single-byte integer.
> > The current branch is good go go:
> > 43cebe1 PROTON-1159: Some improvements for qpid-interop-test: Added operator<<() to byte_array and decimal classes. Fixed UUID print bug.

I rebased your branch, sorry that was rude. You'll need to `git pull -f kvdr-PROTON-1159` to update your local replica.


- Alan


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


On March 17, 2016, 5:34 p.m., Kim van der Riet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44967/
> -----------------------------------------------------------
> 
> (Updated March 17, 2016, 5:34 p.m.)
> 
> 
> Review request for qpid and Alan Conway.
> 
> 
> Bugs: PROTON-1159
>     https://issues.apache.org/jira/browse/PROTON-1159
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> proton::byte_array currently has no method to stream values. To help with qpid-interop-test, a basic version of operator<<() which returns a hex representation of the byte array in the format "0xNNNNNN" of the appropriate length is suggested. The decimal classes use "decimalXX(0xNNNN)" as their format.
> 
> An bug in proton::uuid::operator<<() is also presented in which bytes > 0x80 within the UUID sequence (negative char values) print as ffffffnn rather than nn.
> 
> 
> Diffs
> -----
> 
>   proton-c/bindings/cpp/include/proton/byte_array.hpp fe3f864 
>   proton-c/bindings/cpp/src/decimal.cpp 93fe84c 
>   proton-c/bindings/cpp/src/uuid.cpp f224df7 
> 
> Diff: https://reviews.apache.org/r/44967/diff/
> 
> 
> Testing
> -------
> 
> Works as expected in qpid-interop-test.
> 
> 
> Thanks,
> 
> Kim van der Riet
> 
>


Re: Review Request 44967: Add operator<< to proton::byte_array (and hence to decimal32, decimal64 and decimal128)

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44967/#review124081
-----------------------------------------------------------


Ship it!




This overlaps a bit with some changes I made in my last commit. I rebased onto the latest master and made minor changes:
- hide implementation of ostream<< op in .cpp file
- consistent ostream ops in uuid, binary and byte_array: 
  - all use of ios_guard to restore state flags
  - all use new printable_byte() function to convert a signed/unsigned char value to an unsigned, single-byte integer.
The current branch is good go go:
43cebe1 PROTON-1159: Some improvements for qpid-interop-test: Added operator<<() to byte_array and decimal classes. Fixed UUID print bug.

- Alan Conway


On March 17, 2016, 5:34 p.m., Kim van der Riet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44967/
> -----------------------------------------------------------
> 
> (Updated March 17, 2016, 5:34 p.m.)
> 
> 
> Review request for qpid and Alan Conway.
> 
> 
> Bugs: PROTON-1159
>     https://issues.apache.org/jira/browse/PROTON-1159
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> proton::byte_array currently has no method to stream values. To help with qpid-interop-test, a basic version of operator<<() which returns a hex representation of the byte array in the format "0xNNNNNN" of the appropriate length is suggested. The decimal classes use "decimalXX(0xNNNN)" as their format.
> 
> An bug in proton::uuid::operator<<() is also presented in which bytes > 0x80 within the UUID sequence (negative char values) print as ffffffnn rather than nn.
> 
> 
> Diffs
> -----
> 
>   proton-c/bindings/cpp/include/proton/byte_array.hpp fe3f864 
>   proton-c/bindings/cpp/src/decimal.cpp 93fe84c 
>   proton-c/bindings/cpp/src/uuid.cpp f224df7 
> 
> Diff: https://reviews.apache.org/r/44967/diff/
> 
> 
> Testing
> -------
> 
> Works as expected in qpid-interop-test.
> 
> 
> Thanks,
> 
> Kim van der Riet
> 
>