Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

why read twice in newConnection? #17

Open
donnol opened this issue Jan 13, 2021 · 5 comments
Open

why read twice in newConnection? #17

donnol opened this issue Jan 13, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@donnol
Copy link

donnol commented Jan 13, 2021

func newConnection(d *driver) (*connection, error) {
	c := &connection{}
	c.close()
	conn, err := net.Dial("tcp", fmt.Sprintf("%s:%d", d.Host, d.Port))
	if err != nil {
		return nil, err
	}

	c.closed = false
	c.conn = conn
	c.reader = bufio.NewReader(c.conn)

	err = c.write(fmt.Sprintf("START %s %s", d.channel, d.Password))
	if err != nil {
		return nil, err
	}

	// what is the purpose?
	_, err = c.read()
	_, err = c.read()
	if err != nil {
		return nil, err
	}
	return c, nil
}
@timurgarif
Copy link
Contributor

The first call to c.read() reads CONNECTED <sonic-server vX.Y.Z> line.
The second call reads STARTED search protocol(1) buffer(20000) - the response line to START chan pswd

@donnol
Copy link
Author

donnol commented Jan 25, 2021

@timurgarif Ok, thanks.

But ignoring the err check between the twice read should cause problem if the first read return a err not nil.

@alexisvisco
Copy link
Member

Yes, there is a project of adding some pooling and rewrite the connection mechanism.

For now, I am a little bit busy but maybe the next week I will be working on it.

@donnol
Copy link
Author

donnol commented Jan 25, 2021

That's great, thanks.

Yes, there is a project of adding some pooling and rewrite the connection mechanism.

For now, I am a little bit busy but maybe the next week I will be working on it.

@alexisvisco alexisvisco added the bug Something isn't working label Jan 25, 2021
@krokite
Copy link

krokite commented Aug 20, 2022

Any update on the bug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants