Misusing of defer

[shizhx]

go-mysql/packet/conn.go

Lines 90 to 112 in 1c55c36

	func (c *Conn) ReadPacketReuseMem(dst []byte) ([]byte, error) {
	// Here we use `sync.Pool` to avoid allocate/destroy buffers frequently.
	buf := utils.BytesBufferGet()
	defer utils.BytesBufferPut(buf)
	if err := c.ReadPacketTo(buf); err != nil {
	return nil, errors.Trace(err)
	}
	readBytes := buf.Bytes()
	readSize := len(readBytes)
	var result []byte
	if len(dst) > 0 {
	result = append(dst, readBytes...)
	// if read block is big, do not cache buf any more
	if readSize > utils.TooBigBlockSize {
	buf = nil
	}
	} else {
	if readSize > utils.TooBigBlockSize {
	// if read block is big, use read block as result and do not cache buf any more
	result = readBytes
	buf = nil

defer utils.BytesBufferPut(buf) and

 		// if read block is big, do not cache buf any more 
 		if readSize > utils.TooBigBlockSize { 
 			buf = nil 
 		} 

defer capture buf early, so modify buf after defer not working.

[lance6716]

The reason is

if read block is big, do not cache buf any more

Do you meet some problem with this feature?

[shizhx]

@lance6716 I mean setting buf = nil has no effect after defer utils.BytesBufferPut(buf), utils.BytesBufferPut would always get a non-nil argument.

[lance6716]

Oh thanks for pointing it out. utils.BytesBufferPut should be able to handle too large buf.

Can you open a PR to fix it? I'll check it carefully when review your PR.

[shizhx]

Oh thanks for pointing it out. utils.BytesBufferPut should be able to handle too large buf.

Can you open a PR to fix it? I'll check it carefully when review your PR.

I read the utils.BytesBufferPut implementation, it has handled big buffer:

go-mysql/utils/bytes_buffer_pool.go

Lines 26 to 29 in 1c55c36

	func BytesBufferPut(data *bytes.Buffer) {
	if data == nil || data.Len() > TooBigBlockSize {
	return
	}

so buf = nil has no effect, but the "not cache big buffer" function works as expected.

[skoef]

@shizhx thank you for pointing this out and opening a PR!

May I point out to you that this project is mainly, if not completely, run by volunteers, as do many open source projects. If you want something done, you should really adjust your tone of voice. Calling an honest mistake by a previous contributor "stupid" is certainly not constructive.

[shizhx]

@skoef I am very sorry, I will pay attention in the future.

[shizhx]

@skoef But this project is the most important library about MySQL replication, yet the unit tests are too few, some commits change same line repeatly to fix some 'stupid' issue. So I expect:

1. more unit tests.
2. more carefully review of commit.

[skoef]

@shizhx thank you for understanding! I agree that more unit testing is better and I encourage you to contribute as you seem to be engaged in the project!