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

fix: change UART message buffer size to 35 bytes #79

Merged
merged 1 commit into from Aug 10, 2020

Conversation

JakubVanek
Copy link
Contributor

Previously, the buffer was only 34 bytes large. This would overflow the buffer if an INFO message with 32 byte payload was received. The problem is the INFO command byte that is not counted to the payload size.

I haven't tested the change yet, but I will try to do so.

@JakubVanek JakubVanek marked this pull request as draft August 9, 2020 08:41
@JakubVanek JakubVanek changed the title fix: change UART message buffer size to 35 bytes [testing needed] fix: change UART message buffer size to 35 bytes Aug 9, 2020
@JakubVanek
Copy link
Contributor Author

JakubVanek commented Aug 9, 2020

The change compiles and the color sensor works with it on x86_64 laptop, hopefully nothing broke.

@JakubVanek JakubVanek marked this pull request as ready for review August 9, 2020 09:02
Copy link
Member

@dlech dlech left a comment

Choose a reason for hiding this comment

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

Nice catch!

@@ -66,7 +66,8 @@
#endif

#define EV3_UART_MAX_DATA_SIZE 32
#define EV3_UART_MAX_MESSAGE_SIZE (EV3_UART_MAX_DATA_SIZE + 2)
// extra bytes for: main command byte, INFO command byte, final checksum
Copy link
Member

Choose a reason for hiding this comment

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

Could you please change this to /* */ style comments (it is the Linux kernel style.

@dlech dlech marked this pull request as draft August 10, 2020 21:14
@dlech dlech marked this pull request as ready for review August 10, 2020 21:14
@dlech dlech self-requested a review August 10, 2020 21:15
Copy link
Member

@dlech dlech left a comment

Choose a reason for hiding this comment

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

x

Previously, the buffer was only 34 bytes large.
This would overflow the buffer if an INFO message
with 32 byte payload was received. The problem
is the INFO command byte that is not counted to
the payload size.
@JakubVanek
Copy link
Contributor Author

Thanks! I have now changed the style of the comment.

@dlech dlech merged commit 2c05d90 into ev3dev:ev3dev-buster Aug 10, 2020
@JakubVanek JakubVanek deleted the bugfix/msg-buffer-overflow branch August 10, 2020 21:41
@JakubVanek JakubVanek restored the bugfix/msg-buffer-overflow branch August 10, 2020 21:41
@JakubVanek JakubVanek deleted the bugfix/msg-buffer-overflow branch August 10, 2020 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants