> On May 29, 2014, 10:52 a.m., Rafael Schloming wrote: > > /proton/trunk/proton-c/include/proton/buffer.h, line 35 > > <https://reviews.apache.org/r/21929/diff/2/?file=598509#file598509line35> > > > > It occurs to me that the bytes/wbytes naming isn't super accurate. I > > think pn_bytes_t is fine because it is used to pass around actual bytes, > > however in the case of pn_wbytes_t it is really being used to pass around a > > chunk of memory which (at least initially) may hold no meaningful bytes. > > > > I'm thinking a different name might better reflect the usage and also > > make the split seem less ugly. Maybe pn_mem_t or something along those > > lines?
In the submitted change the struct is called pn_buffer_memory_t - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21929/#review44252 ----------------------------------------------------------- On May 29, 2014, 4:58 a.m., Andrew Stitcher wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21929/ > ----------------------------------------------------------- > > (Updated May 29, 2014, 4:58 a.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/include/proton/buffer.h 1596912 > /proton/trunk/proton-c/include/proton/cproton.i 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 > >
