Skip to content

Changed CircBuffer to take its size as a template parameters for efficientcy. #1044

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 6 commits into from
Apr 23, 2015
Merged

Changed CircBuffer to take its size as a template parameters for efficientcy. #1044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
250ce08
Changed CircBuffer to take its size as a template parameters. This re…
odinthenerd Apr 15, 2015
cad87d1
moved initialization into constructor for compatibility with c++03
odinthenerd Apr 16, 2015
c48e5ea
Update CircBuffer.h
odinthenerd Apr 16, 2015
478aa7e
free memory afer disabling the usb ISR solves race condition which co…
odinthenerd Apr 16, 2015
0209929
Merge branch 'master' of https://github.com/mbedmicro/mbed
odinthenerd Apr 16, 2015
75732d7
Merge branch 'master' of https://github.com/porkybrain/mbed
odinthenerd Apr 16, 2015
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
2 changes: 1 addition & 1 deletion libraries/USBDevice/USBMSD/USBMSD.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ bool USBMSD::connect(bool blocking) {
}

void USBMSD::disconnect() {
USBDevice::disconnect();
//De-allocate MSD page size:
free(page);
page = NULL;
USBDevice::disconnect();
}

void USBMSD::reset() {
Expand Down
18 changes: 4 additions & 14 deletions libraries/USBDevice/USBSerial/CircBuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,10 @@
#ifndef CIRCBUFFER_H
#define CIRCBUFFER_H

template <class T>
template <class T, int Size>
class CircBuffer {
public:
CircBuffer(int length) {
write = 0;
read = 0;
size = length + 1;
buf = (T *)malloc(size * sizeof(T));
};

~CircBuffer() {
free(buf);
}

CircBuffer():write(0), read(0){}
bool isFull() {
return ((write + 1) % size == read);
};
Expand Down Expand Up @@ -66,8 +56,8 @@ class CircBuffer {
private:
volatile uint16_t write;
volatile uint16_t read;
uint16_t size;
T * buf;
static const int size = Size+1; //a modern optimizer should be able to remove this so it uses no ram.
T buf[Size];
Copy link
Contributor

Choose a reason for hiding this comment

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

The code used to malloc the equivalent of Size+1. It seems like it should be "T buf[Size+1];" here. Wouldn't the current code allow the read/write members to overflow the buf array by 1 element?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! It should be T buf[size] (with a small s!), stupid error on my part. As I mentioned somewhere else this circular buffer also has a bunch of race conditions. I have not had the time to do a re write yet, if you are interested since loads and stores are atomic on cortex this should work fine if adapted.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 adam. @porkybrain please send PR with a fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done #1161

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol more like sorry for breaking it

};

#endif
4 changes: 2 additions & 2 deletions libraries/USBDevice/USBSerial/USBSerial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class USBSerial: public USBCDC, public Stream {
* @param connect_blocking define if the connection must be blocked if USB not plugged in
*
*/
USBSerial(uint16_t vendor_id = 0x1f00, uint16_t product_id = 0x2012, uint16_t product_release = 0x0001, bool connect_blocking = true): USBCDC(vendor_id, product_id, product_release, connect_blocking), buf(128){
USBSerial(uint16_t vendor_id = 0x1f00, uint16_t product_id = 0x2012, uint16_t product_release = 0x0001, bool connect_blocking = true): USBCDC(vendor_id, product_id, product_release, connect_blocking){
settingsChangedCallback = 0;
};

Expand Down Expand Up @@ -154,7 +154,7 @@ class USBSerial: public USBCDC, public Stream {

private:
FunctionPointer rx;
CircBuffer<uint8_t> buf;
CircBuffer<uint8_t,128> buf;
void (*settingsChangedCallback)(int baud, int bits, int parity, int stop);
};

Expand Down