Skip to content

Conversation

@jplevyak
Copy link
Contributor

Signed-off-by: John Plevyak [email protected]

@jplevyak jplevyak requested a review from PiotrSikora May 20, 2020 21:21
Copy link
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

LGTM, sans nits.

inline void *WasmBase::allocMemory(uint64_t size, uint64_t *address) {
if (!malloc_) {
return nullptr;
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this? We fail hard if malloc is not exported.

(I'm not saying it's not good to add it just to be safe, I'm just curious why did you add it now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generally I was trying to make the proxy-independent code resilient against unexpected behavior. An integrator could choose to not hard fail on error(). For envoy we throw an exception which we will catch at a higher level, but presumably someone code set a flag.

jplevyak added 2 commits May 21, 2020 01:08
Signed-off-by: John Plevyak <[email protected]>
Signed-off-by: John Plevyak <[email protected]>
@jplevyak jplevyak merged commit f012757 into master May 21, 2020
@mathetake mathetake deleted the wasm-cleanup branch August 6, 2021 09:04
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