Skip to content

Conversation

@yamt
Copy link
Contributor

@yamt yamt commented Aug 26, 2022

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 BHT_ERROR;
}
int ret = os_mutex_init(&tmpbuf_lock);
if (ret == BHT_ERROR) {
Copy link
Collaborator

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

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 (BHT_OK)

if (tmpbuf == NULL) {
return BHT_ERROR;
}
int ret = os_mutex_init(&tmpbuf_lock);
Copy link
Collaborator

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

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

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);
Copy link
Collaborator

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

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.
btw, do you (or anyone) remember why this huge buffer is allocated here?
is it somehow required by the protocol?

@wenyongh wenyongh merged commit 04c1eb3 into bytecodealliance:main Aug 26, 2022
vickiegpt pushed a commit to vickiegpt/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
Some configurations (eg. esp32/nuttx) have limited space for BSS,
0x20000 byte buffer is huge on embedded systems, change to
allocate the buffer dynamically.
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.

3 participants