Skip to content

Replace recursive approach with iterative approach for readPacket() #170

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 13, 2013
Merged

Replace recursive approach with iterative approach for readPacket() #170

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e9e660
Minor changes, avoid recursive approach in readPacket()
cxmcc Nov 11, 2013
745998a
Remove the buffer copying, no longer necessary
cxmcc Nov 11, 2013
7847296
Avoid buffer copying for non-splitting packets
cxmcc Nov 13, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 39 additions & 37 deletions packets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,54 +24,56 @@ import (

// Read packet to buffer 'data'
func (mc *mysqlConn) readPacket() ([]byte, error) {
// Read packet header
data, err := mc.buf.readNext(4)
if err != nil {
errLog.Print(err.Error())
mc.Close()
return nil, driver.ErrBadConn
}
var payload []byte
for {
// Read packet header
data, err := mc.buf.readNext(4)
if err != nil {
errLog.Print(err.Error())
mc.Close()
return nil, driver.ErrBadConn
}

// Packet Length [24 bit]
pktLen := int(uint32(data[0]) | uint32(data[1])<<8 | uint32(data[2])<<16)
// Packet Length [24 bit]
pktLen := int(uint32(data[0]) | uint32(data[1])<<8 | uint32(data[2])<<16)

if pktLen < 1 {
errLog.Print(errMalformPkt.Error())
mc.Close()
return nil, driver.ErrBadConn
}
if pktLen < 1 {
errLog.Print(errMalformPkt.Error())
mc.Close()
return nil, driver.ErrBadConn
}

// Check Packet Sync [8 bit]
if data[3] != mc.sequence {
if data[3] > mc.sequence {
return nil, errPktSyncMul
} else {
return nil, errPktSync
// Check Packet Sync [8 bit]
if data[3] != mc.sequence {
if data[3] > mc.sequence {
return nil, errPktSyncMul
} else {
return nil, errPktSync
}
}
}
mc.sequence++
mc.sequence++

// Read packet body [pktLen bytes]
data, err = mc.buf.readNext(pktLen)
if err != nil {
errLog.Print(err.Error())
mc.Close()
return nil, driver.ErrBadConn
}

isLastPacket := (pktLen < maxPacketSize)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when the packet length of the last packet is exactly maxPacketSize? Is that possible?
I'll look it up now, but if you know, please chime in...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when the packet length of the last packet is exactly maxPacketSize? Is that possible?

No, a packet length of maxPacketSize marks a split packet

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks


// Read packet body [pktLen bytes]
if data, err = mc.buf.readNext(pktLen); err == nil {
if pktLen < maxPacketSize {
// Zero allocations for non-splitting packets
if isLastPacket && payload == nil {
return data, nil
}

// Make a copy since data becomes invalid with the next read
buf := make([]byte, len(data))
copy(buf, data)
payload = append(payload, data...)

// More data
data, err = mc.readPacket()
if err == nil {
return append(buf, data...), nil
if isLastPacket {
return payload, nil
}
}

// err case
mc.Close()
errLog.Print(err.Error())
return nil, driver.ErrBadConn
}

// Write packet buffer 'data'
Expand Down