worryg0d commented on code in PR #1822:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1822#discussion_r1808549641
##########
conn.go:
##########
@@ -694,7 +718,7 @@ func (c *Conn) recv(ctx context.Context) error {
} else if head.stream == -1 {
// TODO: handle cassandra event frames, we shouldnt get any
currently
framer := newFramer(c.compressor, c.version)
- if err := framer.readFrame(c, &head); err != nil {
+ if err := framer.readFrame(r, &head); err != nil {
return err
}
go c.session.handleEvent(framer)
Review Comment:
> This should be changed to r as well right?
You're right. I didn't notice it here.
> I think this is even the case on readUncompressedFrame and
readCompressedFrame... It's using c.r directly but it should be using c so the
additional logic from Conn.Read() is used right?
I didn't even notice that `Conn` implements `io.Reader`. I assumed that
`*bufio.Reader` should be used to read from the socket because I had seen [line
673](https://github.com/apache/cassandra-gocql-driver/blob/953e0df999cabb3f5eef714df9921c00e9f632c2/conn.go#L673C1-L674C1)
where `c.r` is used:
```
head, err := readHeader(c.r, c.headerBuf[:])
```
> I think we should create a type (maybe connReader?) that wraps the
*bufio.Reader object and then move the implementation of Conn.Read() to that
new type so we ensure that we only have one object that implements io.Reader.
This way c.r becomes an object of *connReader instead of *bufio.Reader, Conn no
longer implements io.Reader and everytime we want to actually read from the
socket we have to use the c.r field instead of c directly.
Something like
[this](https://github.com/worryg0d/gocql/commit/44cc2fc9f2344928f261d43c60071db1170d706c)?
It makes sense since it forces us to use `c.r` instead of `Conn` itself.
However, there is `net.Conn` still presented in `Conn`, which may be
accidentally used instead.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
