qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rafael Schloming" <...@apache.org>
Subject Re: Review Request 21929: Split apart the read-only and writable aspects of pn_bytes_t
Date Wed, 28 May 2014 18:28:41 GMT

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



/proton/trunk/proton-c/bindings/perl/perl.i
<https://reviews.apache.org/r/21929/#comment78463>

    I believe there is a central location you can put %ignore pn_bytes_t rather than repeating
it in all the different <lang>.i files. Look for cproton.i underneath include/proton


- Rafael Schloming


On May 27, 2014, 4:32 p.m., Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21929/
> -----------------------------------------------------------
> 
> (Updated May 27, 2014, 4:32 p.m.)
> 
> 
> Review request for qpid, Darryl Pierce, Rafael Schloming, and Ted Ross.
> 
> 
> 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