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

BaseChannel/GetBytes, buffer overrun potential #9

Open
mr23 opened this issue Jul 13, 2013 · 6 comments
Open

BaseChannel/GetBytes, buffer overrun potential #9

mr23 opened this issue Jul 13, 2013 · 6 comments

Comments

@mr23
Copy link

mr23 commented Jul 13, 2013

This needs some improvement to not overrun, and would be more useful if the byte array size was configurable. Also, add BytesToRead to the SP implementation, and use here for performance; push the byte content parsing up. Something like:

public int BufferLen { get; set/* use a method to change buffer size, protected with a lock, or disallow setting when port is opened*/; }

// don't worry about byte values here, just buffer and return the input, parse in caller.
protected byte[] GetBytes(SerialPort port)
{
int position = 0;
int bytesWaiting = port.BytesToRead;
bytesWaiting = Min(bytesWaiting,BufferLen);
byte[] buffer = new byte[bytesWaiting];
var bytesWaiting = port.Read(buffer,0,bytesWaiting); // returns bytes read
return buffer;
}

@nothingmn
Copy link
Owner

Yes, this stuff is definitely a work in progress! The whole Channel/Communication setup needs to be ironed out completely and tested fully....

@mr23
Copy link
Author

mr23 commented Jul 13, 2013

I'd also push the buffer out to the class level and preallocate it, for performance. Less GC and frag.

@mr23
Copy link
Author

mr23 commented Jul 13, 2013

Why close the issue if the code is still fragile/incomplete?

@nothingmn
Copy link
Owner

Hit the wrong button.. :p

@nothingmn nothingmn reopened this Jul 13, 2013
@nothingmn
Copy link
Owner

This should do it...

    protected byte[] GetBytes(SerialPort port)
    {
        byte[] buffer = new byte[port.BytesToRead];
        port.Read(buffer, 0, buffer.Length);
        return buffer;
    }

seems simple enough..

what do you think?

@mr23
Copy link
Author

mr23 commented Jul 13, 2013

Could run out of memory quickly. Think 57.6k for example, 1+ seconds. Otherwise it is simply correct.
In my testing on the emulator, despite either timer polling or DataReceived, sometimes a stall occurred for a few seconds.
No doubt with an OS the AGENT may too.
Safer to use a limit, allowing User to tweak it.

Sent from my Kindle Fire


From: Rob Chartier notifications@github.com
Sent: Fri Jul 12 22:57:50 CST 2013
To: "nothingmn/AGENT.Contrib" AGENT.Contrib@noreply.github.com
Cc: Chris Meier mr23@comcast.net
Subject: Re: [AGENT.Contrib] BaseChannel/GetBytes, buffer overrun potential (#9)

This should do it...

protected byte[] GetBytes(SerialPort port) { byte[] buffer = new byte[port.BytesToRead]; port.Read(buffer, 0, buffer.Length); return buffer; }

seems simple enough..

what do you think?


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants