Make atomvm:add_avm_pack_file/2 return a meaningful error

Function was raising an out of memory error for all kind of errors, make it return specific errors so we can have better error handling. Signed-off-by: Davide Bettio <[email protected]>
atomvm · Jul 22, 2023 · b2f582b · b2f582b
1 parent b494bc4
commit b2f582b
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 34 deletions.
diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c
@@ -3445,6 +3445,38 @@ static term nif_atomvm_add_avm_pack_binary(Context *ctx, int argc, term argv[])
     return OK_ATOM;
 }
 
+static term open_avm_error_tuple(Context *ctx, enum OpenAVMResult result)
+{
+    term reason = UNDEFINED_ATOM;
+    switch (result) {
+        case AVM_OPEN_FAILED_ALLOC:
+            RAISE_ERROR(OUT_OF_MEMORY_ATOM);
+            break;
+        case AVM_OPEN_INVALID:
+            reason = globalcontext_make_atom(ctx->global, ATOM_STR("\xB", "invalid_avm"));
+            break;
+        case AVM_OPEN_CANNOT_OPEN:
+            reason = globalcontext_make_atom(ctx->global, ATOM_STR("\xB", "cannot_open"));
+            break;
+        case AVM_OPEN_CANNOT_READ:
+            reason = globalcontext_make_atom(ctx->global, ATOM_STR("\xB", "cannot_read"));
+            break;
+        case AVM_OPEN_NOT_SUPPORTED:
+            reason = globalcontext_make_atom(ctx->global, ATOM_STR("\xD", "not_supported"));
+            break;
+        case AVM_OPEN_OK:
+            __builtin_unreachable();
+    }
+    if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) {
+        RAISE_ERROR(OUT_OF_MEMORY_ATOM);
+    }
+    term error_tuple = term_alloc_tuple(2, &ctx->heap);
+    term_put_tuple_element(result, 0, ERROR_ATOM);
+    term_put_tuple_element(result, 1, reason);
+
+    return error_tuple;
+}
+
 // AtomVM extension
 static term nif_atomvm_add_avm_pack_file(Context *ctx, int argc, term argv[])
 {
@@ -3463,10 +3495,11 @@ static term nif_atomvm_add_avm_pack_file(Context *ctx, int argc, term argv[])
         RAISE_ERROR(BADARG_ATOM);
     }
 
-    struct AVMPackData *avmpack_data = sys_open_avm_from_file(ctx->global, abs);
-    if (IS_NULL_PTR(avmpack_data)) {
+    struct AVMPackData *avmpack_data;
+    enum OpenAVMResult result = sys_open_avm_from_file(ctx->global, abs, &avmpack_data);
+    if (UNLIKELY(result != AVM_OPEN_OK)) {
         free(abs);
-        RAISE_ERROR(OUT_OF_MEMORY_ATOM);
+        return open_avm_error_tuple(ctx, result);
     }
 
     term name = interop_kv_get_value_default(opts, ATOM_STR("\x4", "name"), UNDEFINED_ATOM, ctx->global);

diff --git a/src/libAtomVM/sys.h b/src/libAtomVM/sys.h
@@ -40,12 +40,24 @@ extern "C" {
 #include <stdint.h>
 #include <time.h>
 
+struct AVMPackData;
+
 enum
 {
     SYS_POLL_EVENTS_DO_NOT_WAIT = 0,
     SYS_POLL_EVENTS_WAIT_FOREVER = -1
 };
 
+enum OpenAVMResult
+{
+    AVM_OPEN_OK = 0,
+    AVM_OPEN_FAILED_ALLOC = 1,
+    AVM_OPEN_INVALID = 2,
+    AVM_OPEN_CANNOT_OPEN = 3,
+    AVM_OPEN_CANNOT_READ = 4,
+    AVM_OPEN_NOT_SUPPORTED = 5
+};
+
 /**
  * @brief Event listener
  *
@@ -182,7 +194,8 @@ void sys_signal(GlobalContext *glb);
 
 #endif
 
-struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *path);
+enum OpenAVMResult sys_open_avm_from_file(
+    GlobalContext *global, const char *path, struct AVMPackData **data);
 
 /**
  * @brief gets wall clock time

diff --git a/src/platforms/emscripten/src/lib/sys.c b/src/platforms/emscripten/src/lib/sys.c
@@ -412,7 +412,8 @@ static void *load_or_fetch_file(const char *path, emscripten_fetch_t **fetch, si
     return (void *) (*fetch)->data;
 }
 
-struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *path)
+enum OpenAVMResult sys_open_avm_from_file(
+    GlobalContext *global, const char *path, struct AVMPackData **avm_data)
 {
     TRACE("sys_open_avm_from_file: Going to open: %s\n", path);
 
@@ -421,7 +422,7 @@ struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *pa
     emscripten_fetch_t *fetch = NULL;
     void *data = load_or_fetch_file(path, &fetch, NULL);
     if (IS_NULL_PTR(data)) {
-        return NULL;
+        return AVM_OPEN_CANNOT_OPEN;
     }
 
     struct ConstAVMPack *const_avm = malloc(sizeof(struct ConstAVMPack));
@@ -431,12 +432,13 @@ struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *pa
         } else {
             free(data);
         }
-        return NULL;
+        return AVM_OPEN_FAILED_ALLOC;
     }
     avmpack_data_init(&const_avm->base, &const_avm_pack_info);
     const_avm->base.data = (const uint8_t *) data;
 
-    return &const_avm->base;
+    *avm_data = &const_avm->base;
+    return AVM_OPEN_OK;
 }
 
 Module *sys_load_module_from_file(GlobalContext *global, const char *path)

diff --git a/src/platforms/esp32/components/avm_sys/sys.c b/src/platforms/esp32/components/avm_sys/sys.c
@@ -257,7 +257,7 @@ static void esp32_part_avm_pack_destructor(struct AVMPackData *obj, GlobalContex
     free(obj);
 }
 
-struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *path)
+enum OpenAVMResult sys_open_avm_from_file(GlobalContext *global, const char *path, struct AVMPackData **data)
 {
     TRACE("sys_open_avm_from_file: Going to open: %s\n", path);
 
@@ -271,16 +271,15 @@ struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *pa
         const char *part_name = path + parts_by_name_len;
         const void *part_data = esp32_sys_mmap_partition(part_name, &part_handle, &size);
         if (IS_NULL_PTR(part_data)) {
-            return NULL;
+            return AVM_OPEN_CANNOT_OPEN;
         }
         if (UNLIKELY(!avmpack_is_valid(part_data, size))) {
-            return NULL;
+            return AVM_OPEN_INVALID;
         }
 
         struct ESP32PartAVMPack *part_avm = malloc(sizeof(struct ESP32PartAVMPack));
         if (IS_NULL_PTR(part_avm)) {
-            // use esp_partition_munmap
-            return NULL;
+            return AVM_OPEN_FAILED_ALLOC;
         }
         avmpack_data_init(&part_avm->base, &esp32_part_avm_pack_info);
         part_avm->base.data = part_data;
@@ -290,46 +289,47 @@ struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *pa
     } else {
         struct stat file_stats;
         if (UNLIKELY(stat(path, &file_stats) < 0)) {
-            return NULL;
+            return AVM_OPEN_CANNOT_OPEN;
         }
         // check int max
         int size = file_stats.st_size;
 
         void *file_data = malloc(size);
         if (IS_NULL_PTR(file_data)) {
-            return NULL;
+            return AVM_OPEN_FAILED_ALLOC;
         }
 
         FILE *avm_file = fopen(path, "r");
         if (UNLIKELY(!avm_file)) {
             free(file_data);
-            return NULL;
+            return AVM_OPEN_CANNOT_OPEN;
         }
 
         int bytes_read = fread(file_data, 1, size, avm_file);
         fclose(avm_file);
 
         if (UNLIKELY(bytes_read != size)) {
             free(file_data);
-            return NULL;
+            return AVM_OPEN_CANNOT_READ;
         }
 
         if (UNLIKELY(!avmpack_is_valid(file_data, size))) {
             free(file_data);
-            return NULL;
+            return AVM_OPEN_INVALID;
         }
 
         struct InMemoryAVMPack *in_memory_avm = malloc(sizeof(struct InMemoryAVMPack));
         if (IS_NULL_PTR(avmpack_data)) {
             free(file_data);
-            return NULL;
+            return AVM_OPEN_FAILED_ALLOC;
         }
         avmpack_data_init(&in_memory_avm->base, &in_memory_avm_pack_info);
         in_memory_avm->base.data = file_data;
         avmpack_data = &in_memory_avm->base;
     }
 
-    return avmpack_data;
+    *data = avmpack_data;
+    return AVM_OPEN_OK;
 }
 
 Module *sys_load_module_from_file(GlobalContext *global, const char *path)

diff --git a/src/platforms/generic_unix/lib/sys.c b/src/platforms/generic_unix/lib/sys.c
@@ -365,32 +365,32 @@ uint64_t sys_monotonic_millis()
     return (ts.tv_nsec / 1000000UL) + (ts.tv_sec * 1000UL);
 }
 
-struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *path)
+enum OpenAVMResult sys_open_avm_from_file(
+    GlobalContext *global, const char *path, struct AVMPackData **data)
 {
     TRACE("sys_open_avm_from_file: Going to open: %s\n", path);
 
     UNUSED(global);
 
     MappedFile *mapped = mapped_file_open_beam(path);
     if (IS_NULL_PTR(mapped)) {
-        return NULL;
+        return AVM_OPEN_CANNOT_OPEN;
     }
     if (UNLIKELY(!avmpack_is_valid(mapped->mapped, mapped->size))) {
-        fprintf(stderr, "%s is not a valid AVM Pack file.\n", path);
-        return NULL;
+        return AVM_OPEN_INVALID;
     }
 
     struct MappedFileAVMPack *avmpack_data = malloc(sizeof(struct MappedFileAVMPack));
     if (IS_NULL_PTR(avmpack_data)) {
-        fprintf(stderr, "Memory error: Cannot allocate AVMPackData.\n");
         mapped_file_close(mapped);
-        return NULL;
+        return AVM_OPEN_FAILED_ALLOC;
     }
     avmpack_data_init(&avmpack_data->base, &mapped_file_avm_pack_info);
     avmpack_data->base.data = mapped->mapped;
     avmpack_data->mapped = mapped;
 
-    return &avmpack_data->base;
+    *data = &avmpack_data->base;
+    return AVM_OPEN_OK;
 }
 
 static void mapped_file_avm_pack_destructor(struct AVMPackData *obj, GlobalContext *global)

diff --git a/src/platforms/generic_unix/main.c b/src/platforms/generic_unix/main.c
@@ -54,8 +54,8 @@ int main(int argc, char **argv)
     for (int i = 1; i < argc; ++i) {
         const char *ext = strrchr(argv[i], '.');
         if (ext && strcmp(ext, ".avm") == 0) {
-            struct AVMPackData *avmpack_data = sys_open_avm_from_file(glb, argv[i]);
-            if (IS_NULL_PTR(avmpack_data)) {
+            struct AVMPackData *avmpack_data;
+            if (UNLIKELY(sys_open_avm_from_file(glb, argv[i], &avmpack_data) != AVM_OPEN_OK)) {
                 fprintf(stderr, "Failed opening %s.\n", argv[i]);
                 return EXIT_FAILURE;
             }

diff --git a/src/platforms/rp2040/src/lib/sys.c b/src/platforms/rp2040/src/lib/sys.c
@@ -135,13 +135,14 @@ uint64_t sys_monotonic_millis()
     return usec / 1000;
 }
 
-struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *path)
+enum OpenAVMResult sys_open_avm_from_file(
+    GlobalContext *global, const char *path, struct AVMPackData **data)
 {
     UNUSED(global);
     UNUSED(path);
 
-    // No file support on pico.
-    return NULL;
+    // TODO
+    return AVM_OPEN_NOT_SUPPORTED;
 }
 
 Module *sys_load_module_from_file(GlobalContext *global, const char *path)

diff --git a/src/platforms/stm32/src/lib/sys.c b/src/platforms/stm32/src/lib/sys.c
@@ -101,12 +101,13 @@ uint64_t sys_monotonic_millis()
     return system_millis;
 }
 
-struct AVMPackData *sys_open_avm_from_file(GlobalContext *global, const char *path)
+enum OpenAVMResult sys_open_avm_from_file(
+    GlobalContext *global, const char *path, struct AVMPackData **data)
 {
     TRACE("sys_open_avm_from_file: Going to open: %s\n", path);
 
     // TODO
-    return NULL;
+    return AVM_OPEN_NOT_SUPPORTED;
 }
 
 Module *sys_load_module_from_file(GlobalContext *global, const char *path)