Skip to content

WIP: new externalterm API #1672

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

bettio
Copy link
Collaborator

@bettio bettio commented May 22, 2025

WIP, THIS PR IS STILL MEANT ONLY FOR DISCUSSION

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

Signed-off-by: Davide Bettio <[email protected]>
@bettio
Copy link
Collaborator Author

bettio commented May 22, 2025

  1. Remove externalterm_from_const_literal. This API is used only from module_load_literal and it doesn't allow to handle out of memory in a graceful way. src/libAtomVM/opcodesswitch.h can already handle this kind of error, but we are aborting in module.c

  2. We still miss support for a safe option in binary_to_term

  3. Add a new API that can be more flexible

enum ExternalTermResult
{
OutOfMem,
[...]
};

enum ExternalTermOpts
{
    Copy,
    Safe
    ...
};

ExternalTermResult result externalterm_heap_from_buf(const void *buf, size_t buf_size, ExternalTermOpts opts, Heap *out_heap, term *out_term, GlobalContext *glb);

ExternalTermResult result externalterm_term_from_buf(const void *buf, size_t buf_size, ExternalTermOpts opts, term roots[], term roots_count, term *out_term, Context *ctx);

Also other opinions:
4. I think an API that allows to deal with any raw buffer containing an external term can be valuable for different use case, so we must make sure our API will support it.
5. I think that our current API with the index parameter for the binary argument might be tricky maybe.

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.

1 participant