-
Notifications
You must be signed in to change notification settings - Fork 749
debug-engine: allocate "tmpbuf" dynamically #1423
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
Conversation
| return BHT_ERROR; | ||
| } | ||
| int ret = os_mutex_init(&tmpbuf_lock); | ||
| if (ret == BHT_ERROR) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had better use ret != BHT_SUCCESS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done (BHT_OK)
| if (tmpbuf == NULL) { | ||
| return BHT_ERROR; | ||
| } | ||
| int ret = os_mutex_init(&tmpbuf_lock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had better define ret at the beginning of the function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Some configurations (eg. esp32/nuttx) have limited space for BSS. 0x20000 byte buffer is huge on embedded systems. I guess in general malloc heap tends to have less limitations. And probably more importantly, this change makes it allocated only when the feature (debug engine) is actually used.
| { | ||
| return os_mutex_init(&tmpbuf_lock); | ||
| int ret; | ||
| tmpbuf = wasm_runtime_malloc(MAX_PACKET_SIZE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On some constraint devices, there is a great possibility of malloc failure for such a large space, had better add LOG_ERROR("Allocate debug package buffer failed");, otherwise the developer may not know what happened
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
btw, do you (or anyone) remember why this huge buffer is allocated here?
is it somehow required by the protocol?
Some configurations (eg. esp32/nuttx) have limited space for BSS, 0x20000 byte buffer is huge on embedded systems, change to allocate the buffer dynamically.
Some configurations (eg. esp32/nuttx) have limited space for BSS.
0x20000 byte buffer is huge on embedded systems.
I guess in general malloc heap tends to have less limitations.
And probably more importantly, this change makes it allocated only
when the feature (debug engine) is actually used.