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 bigbuf allocators (tracing + malloc) overwriting each other #2728

Merged
merged 1 commit into from
Jan 25, 2025

Conversation

shuffle2
Copy link
Contributor

  • BigBuf.c: use s_ prefix for statics
  • BigBuf_Clear_ext already calls clear_trace, so remove extra calls
  • add some sanity checking of allocator args
  • dont compare PDC_RNCR to false

Copy link

You are welcome to add an entry to the CHANGELOG.md as well

@@ -138,8 +138,9 @@ void BigBuf_Clear_keep_EM(void) {
uint8_t *BigBuf_malloc(uint16_t chunksize) {
chunksize = (chunksize + BIGBUF_ALIGN_BYTES - 1) & BIGBUF_ALIGN_MASK; // round up to next multiple of 4

if (s_bigbuf_hi < chunksize) {
return NULL; // no memory left
if (s_bigbuf_hi - s_trace_len < chunksize || chunksize == 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the actual fix should just be the addition of s_bigbuf_hi - s_trace_len < chunksize here.

@@ -1381,12 +1380,12 @@ static int Get14443bAnswerFromTag(uint8_t *response, uint16_t max_len, uint32_t
if (AT91C_BASE_SSC->SSC_SR & (AT91C_SSC_ENDRX)) {

// primary buffer was stopped
if (AT91C_BASE_PDC_SSC->PDC_RCR == false) {
if (!AT91C_BASE_PDC_SSC->PDC_RCR) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code style, we are moving away from !expression since its easy to miss its a negation.
if (expression)
and
if (expression == false)

is our current code style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is == 0 ok in this case (it's an integer field, not boolean)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure.

@iceman1001
Copy link
Collaborator

Nice finding this bug!

And yes, I know our contributing.md file hasn't been updated with the if (exp) and if (exp ==false) comparison style.
We still moving to it slowly.

* BigBuf.c: use s_ prefix for statics
* BigBuf_Clear_ext already calls clear_trace, so remove extra calls
* add some sanity checking of allocator args
* dont compare PDC_RNCR to false
@iceman1001 iceman1001 merged commit ecea5db into RfidResearchGroup:master Jan 25, 2025
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants