qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Stitcher" <astitc...@apache.org>
Subject Review Request 21929: Split apart the read-only and writable aspects of pn_bytes_t
Date Tue, 27 May 2014 16:30:28 GMT

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

Review request for qpid, Darryl Pierce and Rafael Schloming.


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


Repository: qpid


Description
-------

Added a new type pn_wbytes_t and added a new buffer method pn_buffer_wbytes() to return a
writeable area of a buffer. This is what is necessary to make the pn_bytes_t struct read-only.
pn_wbytes_t isonly used internally so doesn't need to be exposed in the API.

Points/Questions:

1. It looks like pn_buffer_t itself is a purely internal API - perhaps it shouldn't be in
the exported include files and doesn't need the PN_EXTERN markings.

2. As the bindings have in/out typemaps for pn_bytes_t I have removed the wrapping of the
pn_bytes_t struct as it doesn't seem necessary and now it could cause problems as swig no
longer generates free()s dues to the const char* member I added.

3. I removed pn_bytes_dup()) because it is never used internally and it is now unnecessary
because none of the calls that takes a pn_bytes_t stores it away or tries to modify the bytes
- This is an API breakage and it could be added back easily as a null function.


Diffs
-----

  /proton/trunk/proton-c/bindings/perl/perl.i 1596912 
  /proton/trunk/proton-c/bindings/php/php.i 1596912 
  /proton/trunk/proton-c/bindings/python/python.i 1596912 
  /proton/trunk/proton-c/bindings/ruby/ruby.i 1596912 
  /proton/trunk/proton-c/include/proton/buffer.h 1596912 
  /proton/trunk/proton-c/include/proton/types.h 1596912 
  /proton/trunk/proton-c/src/buffer.c 1596912 
  /proton/trunk/proton-c/src/codec/codec.c 1596912 
  /proton/trunk/proton-c/src/dispatcher/dispatcher.c 1596912 
  /proton/trunk/proton-c/src/messenger/messenger.c 1596912 
  /proton/trunk/proton-c/src/sasl/sasl.c 1596912 
  /proton/trunk/proton-c/src/types.c 1596912 

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


Testing
-------

ctest


Thanks,

Andrew Stitcher


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message