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

driver: assure required buffer size #5239

Merged
merged 1 commit into from
Apr 4, 2016

Conversation

OlegHahm
Copy link
Member

@OlegHahm OlegHahm commented Apr 3, 2016

The cpuid buffer is also used as a temporary buffer to store the EUI-64 of the transceiver, so we need to make sure that it is always big enough.
May be padded with zeroes for smaller CPUIDs.

The cpuid buffer is also used as a temporary buffer to store the EUI-64 of the transceiver, so we need to make sure that it is always big enough.
May be padded with zeroes for smaller CPUIDs.
@OlegHahm OlegHahm added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Area: drivers Area: Device drivers labels Apr 3, 2016
@OlegHahm OlegHahm added this to the Release 2016.04 milestone Apr 3, 2016
@OlegHahm
Copy link
Member Author

OlegHahm commented Apr 3, 2016

Btw. I found this bug, when I tried to use some 802.15.4 stuff on native and copied code from the at86rf231 driver.

@OlegHahm
Copy link
Member Author

OlegHahm commented Apr 3, 2016

added another commit that get rid of all the literals for the address length that I could find.

@OlegHahm OlegHahm added the Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation label Apr 3, 2016
@miri64
Copy link
Member

miri64 commented Apr 3, 2016

Erm, I might have not gotten the whole patch yet (just glanced over it) but this PR seems to be more about addresses rather then about buffers.

@OlegHahm
Copy link
Member Author

OlegHahm commented Apr 3, 2016

Just look at 6f58862 - that's the actual change. The rest is just cleanup.

/* in case CPUID_LEN < 8, fill missing bytes with zeros */
for (int i = CPUID_LEN; i < 8; i++) {
for (int i = CPUID_LEN; i < IEEE802154_LONG_ADDRESS_LEN; i++) {
cpuid[i] = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Why not use memset here? (sorry, I know it's not your code)

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, should be fixed in a different PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

@OlegHahm
Copy link
Member Author

OlegHahm commented Apr 4, 2016

I can also split up this PR in two. One with the fix and one with the cleanup, if you prefer.

@miri64
Copy link
Member

miri64 commented Apr 4, 2016

Yes, I would prefer that.

@OlegHahm
Copy link
Member Author

OlegHahm commented Apr 4, 2016

Okay, see #5242 for the cleanup.

@miri64
Copy link
Member

miri64 commented Apr 4, 2016

ACK.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 4, 2016
@OlegHahm OlegHahm merged commit 53f33c4 into RIOT-OS:master Apr 4, 2016
@OlegHahm OlegHahm deleted the cpuid_smaller_8 branch April 4, 2016 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants