[ 
https://issues.apache.org/jira/browse/GEODE-10098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17501552#comment-17501552
 ] 

ASF GitHub Bot commented on GEODE-10098:
----------------------------------------

mreddington commented on a change in pull request #943:
URL: https://github.com/apache/geode-native/pull/943#discussion_r819939404



##########
File path: cppcache/src/TcrConnection.cpp
##########
@@ -531,11 +532,9 @@ ConnErrType TcrConnection::sendData(const char* buffer, 
size_t length,
   return CONN_NOERR;
 }
 
-char* TcrConnection::sendRequest(const char* buffer, size_t len,
-                                 size_t* recvLen,
-                                 std::chrono::microseconds sendTimeoutSec,
-                                 std::chrono::microseconds receiveTimeoutSec,
-                                 int32_t request) {
+std::tuple<std::unique_ptr<char[]>, std::size_t> TcrConnection::sendRequest(

Review comment:
       @pivotal-jbarrett You know, when I look back at how I implemented this, 
I made this type assuming `DataInput` took ownership of the pointer. I later 
found out it doesn't, but then I guess I forgot to scrap this in favor of 
`vector`. I know @pdxcodemonkey mentioned `vector` at some point when he asked 
me to do this, but the words didn't leave a lasting impression. An oversight; 
`vector` is better.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> TcrConnection::readMessage should not be explicitly allocating memory
> ---------------------------------------------------------------------
>
>                 Key: GEODE-10098
>                 URL: https://issues.apache.org/jira/browse/GEODE-10098
>             Project: Geode
>          Issue Type: Improvement
>          Components: native client
>            Reporter: Blake Bender
>            Priority: Major
>              Labels: pull-request-available
>
> This method calls new to read an array of bytes, then returns it to the 
> caller, whose responsibility is to delete it (what the heck???).  Even 
> better, the memory is deleted in a call to TcrMessage::setData, so not even 
> in the same class.  If this memory was a std::vector<int8_t>, we could 
> probably take advantage of move semantics and maybe even improve performance 
> a bit, in addition to avoiding potential leaks and weirdness...



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to