yurishkuro commented on a change in pull request #2292:
URL: https://github.com/apache/thrift/pull/2292#discussion_r540574786
##########
File path: lib/go/thrift/binary_protocol.go
##########
@@ -482,35 +480,37 @@ func (p *TBinaryProtocol) readAll(ctx context.Context,
buf []byte) (err error) {
const readLimit = 32768
func (p *TBinaryProtocol) readStringBody(size int32) (value string, err error)
{
+ buf, err := safeReadBytes(size, p.trans, p.buffer[:], readLimit)
+ return buf.String(), NewTProtocolException(err)
+}
+
+// This function is shared between TBinaryProtocol and TCompactProtocol.
+func safeReadBytes(size int32, trans io.Reader, buffer []byte, singleReadLimit
int) (*bytes.Buffer, error) {
Review comment:
I would rather return just the slice `[]byte`, which would be
heap-allocated anyway, and keep the `bytes.Buffer` hidden and allocated on the
stack, as it was previously.
Also the semantics of this function is a bit confusing - what is the role of
`buffer` argument?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
