Update atom_table API and allow for longer atoms

Signed-off-by: Paul Guyot <pguyot@kallisys.net>

Author: pguyot

src/libAtomVM/atom_table.c

@@ -61,24 +61,61 @@ void atom_table_destroy(struct AtomTable *table);
 
 size_t atom_table_count(struct AtomTable *table);
 
-AtomIndex atom_table_ensure_atom(struct AtomTable *table, AtomString string, enum AtomTableCopyOpt opts);
-
-// This function is deprecated and it will be removed.
-// atom strings should be copied to caller owned buffers.
-AtomString atom_table_get_atom_string(struct AtomTable *table, AtomIndex index);
+AtomIndex atom_table_ensure_atom(struct AtomTable *table, const uint8_t *atom_data, size_t atom_len, enum AtomTableCopyOpt opts);
 
 AtomIndex atom_table_get_index(struct AtomTable *table, AtomString string);
+AtomIndex atom_table_get_index_from_cstring(struct AtomTable *table, const char *name);
 
-enum AtomTableEnsureAtomsResult atom_table_ensure_atoms(struct AtomTable *table, const void *atoms, int count,
+enum AtomTableEnsureAtomsResult atom_table_ensure_atoms(struct AtomTable *table, const void *atoms, size_t count,
     AtomIndex *translate_table, enum EnsureAtomsOpt opts);
 
+bool atom_table_is_equal_to_atom_string(
+    struct AtomTable *table, AtomIndex t_atom_index, AtomString atom_string);
 int atom_table_cmp_using_atom_index(
-    struct AtomTable *table, int t_atom_index, int other_atom_index);
-atom_ref_t atom_table_get_atom_ptr_and_len(struct AtomTable *table, AtomIndex index, size_t *out_len);
-bool atom_table_is_atom_ref_ascii(struct AtomTable *table, atom_ref_t atom);
-void atom_table_write_bytes(struct AtomTable *table, atom_ref_t atom, size_t buf_len, void *outbuf);
-void atom_table_write_cstring(
-    struct AtomTable *table, atom_ref_t atom, size_t buf_len, char *outbuf);
+    struct AtomTable *table, AtomIndex t_atom_index, AtomIndex other_atom_index);
+
+/**
+ * @brief Allocate a new C string with malloc with the representation of
+ * an atom
+ *
+ * @param   table atom table
+ * @param   atom_index index of the atom to get the representation of
+ * @return a newly allocated string with the representation of the atom or
+ * NULL if allocation failed or atom index doesn't exist
+ */
+char *atom_table_atom_to_new_cstring(struct AtomTable *table, AtomIndex atom_index);
+
+/**
+ * @brief Get a pointer to the character data of a given atom.
+ *
+ * @details returned pointer is not null terminated
+ *
+ * @param   table atom table
+ * @param   atom_index index of the atom to get the representation of
+ * @param   out_len on output, size of the character data
+ */
+const uint8_t *atom_table_get_atom_string(struct AtomTable *table, AtomIndex index, size_t *out_len);
+
+/**
+ * @brief Write module:function/arity to the supplied buffer.
+ *
+ * @details Write module:function/arity to the supplied buffer.  This function will abort
+ *          if the written module, function, and arity are longer than the supplied
+ *          buffer size.
+ * @param   buf the buffer to write into
+ * @param   buf_size the amount of room in the buffer
+ * @param   module the module name
+ * @param   function the function name
+ * @param   arity the function arity
+ */
+static inline void atom_table_write_mfa(struct AtomTable *table, char *buf, size_t buf_size, AtomIndex module, AtomIndex function, unsigned int arity)
+{
+    size_t module_name_len;
+    const uint8_t *module_name = atom_table_get_atom_string(table, module, &module_name_len);
+    size_t function_name_len;
+    const uint8_t *function_name = atom_table_get_atom_string(table, function, &function_name_len);
+    atom_write_mfa(buf, buf_size, module_name_len, module_name, function_name_len, function_name, arity);
+}
 
 #ifdef __cplusplus
 }

src/libAtomVM/atom_table.h

@@ -1579,34 +1579,30 @@ static term list_to_atom(Context *ctx, term a_list, bool create_new, term *error
     }
 
     int ok;
-    char *atom_string = interop_list_to_utf8_string(a_list, &ok);
+    avm_int_t len = term_list_length(a_list, &ok);
     if (UNLIKELY(!ok)) {
-        *error_reason = OUT_OF_MEMORY_ATOM;
+        *error_reason = BADARG_ATOM;
         return term_invalid_term();
     }
-    int atom_string_len = strlen(atom_string);
-    if (UNLIKELY(atom_string_len > 255)) {
-        free(atom_string);
+    if (len > 255) {
         *error_reason = SYSTEM_LIMIT_ATOM;
         return term_invalid_term();
     }
 
-    AtomString atom = malloc(atom_string_len + 1);
-    if (IS_NULL_PTR(atom)) {
-        free(atom_string);
+    char *atom_string = interop_list_to_utf8_string(a_list, &ok);
+    if (UNLIKELY(!ok)) {
         *error_reason = OUT_OF_MEMORY_ATOM;
         return term_invalid_term();
     }
-    ((uint8_t *) atom)[0] = atom_string_len;
-    memcpy(((char *) atom) + 1, atom_string, atom_string_len);
-    free(atom_string);
+    size_t atom_string_len = strlen(atom_string);
 
     enum AtomTableCopyOpt atom_opts = AtomTableCopyAtom;
     if (!create_new) {
         atom_opts |= AtomTableAlreadyExisting;
     }
-    long global_atom_index = atom_table_ensure_atom(ctx->global->atom_table, atom, atom_opts);
-    free((void *) atom);
+    AtomIndex global_atom_index = atom_table_ensure_atom(ctx->global->atom_table, (const uint8_t *) atom_string, atom_string_len, atom_opts);
+    free(atom_string);
+
     if (UNLIKELY(global_atom_index == ATOM_TABLE_NOT_FOUND)) {
         *error_reason = BADARG_ATOM;
         return term_invalid_term();
@@ -1650,10 +1646,6 @@ term binary_to_atom(Context *ctx, term a_binary, term encoding, bool create_new,
 
     const char *atom_string = term_binary_data(a_binary);
     size_t atom_string_len = term_binary_size(a_binary);
-    if (UNLIKELY(atom_string_len > 255)) {
-        *error_reason = SYSTEM_LIMIT_ATOM;
-        return term_invalid_term();
-    }
 
     bool encode_latin1_to_utf8 = false;
     if (UNLIKELY((encoding == LATIN1_ATOM)
@@ -1665,48 +1657,50 @@ term binary_to_atom(Context *ctx, term a_binary, term encoding, bool create_new,
         return term_invalid_term();
     }
 
-    AtomString atom;
+    const uint8_t *atom_data;
+    size_t atom_data_len;
+    uint8_t *buf = NULL;
     if (LIKELY(!encode_latin1_to_utf8)) {
         if (UNLIKELY(!unicode_is_valid_utf8_buf((const uint8_t *) atom_string, atom_string_len))) {
             *error_reason = BADARG_ATOM;
             return term_invalid_term();
         }
-        atom = malloc(atom_string_len + 1);
-        if (IS_NULL_PTR(atom)) {
-            *error_reason = OUT_OF_MEMORY_ATOM;
+        size_t len = unicode_buf_utf8_len((const uint8_t *) atom_string, atom_string_len);
+        if (UNLIKELY(len > 255)) {
+            *error_reason = SYSTEM_LIMIT_ATOM;
             return term_invalid_term();
         }
-        ((uint8_t *) atom)[0] = atom_string_len;
-        memcpy(((char *) atom) + 1, atom_string, atom_string_len);
+
+        atom_data = (const uint8_t *) atom_string;
+        atom_data_len = atom_string_len;
     } else {
+        if (UNLIKELY(atom_string_len > 255)) {
+            *error_reason = SYSTEM_LIMIT_ATOM;
+            return term_invalid_term();
+        }
+
         // * 2 is the worst case size
         size_t buf_len = atom_string_len * 2;
-        atom = malloc(buf_len + 1);
-        if (IS_NULL_PTR(atom)) {
+        buf = malloc(buf_len);
+        if (IS_NULL_PTR(buf)) {
             *error_reason = OUT_OF_MEMORY_ATOM;
             return term_invalid_term();
         }
-        uint8_t *atom_data = ((uint8_t *) atom) + 1;
-        size_t out_pos = 0;
+        atom_data = buf;
+        atom_data_len = 0;
         for (size_t i = 0; i < atom_string_len; i++) {
             size_t out_size;
-            bitstring_utf8_encode(((uint8_t) atom_string[i]), &atom_data[out_pos], &out_size);
-            out_pos += out_size;
-        }
-        if (out_pos > 255) {
-            free((void *) atom);
-            *error_reason = SYSTEM_LIMIT_ATOM;
-            return term_invalid_term();
+            bitstring_utf8_encode(((uint8_t) atom_string[i]), &buf[atom_data_len], &out_size);
+            atom_data_len += out_size;
         }
-        ((uint8_t *) atom)[0] = out_pos;
     }
 
     enum AtomTableCopyOpt atom_opts = AtomTableCopyAtom;
     if (!create_new) {
         atom_opts |= AtomTableAlreadyExisting;
     }
-    long global_atom_index = atom_table_ensure_atom(ctx->global->atom_table, atom, atom_opts);
-    free((void *) atom);
+    AtomIndex global_atom_index = atom_table_ensure_atom(ctx->global->atom_table, atom_data, atom_data_len, atom_opts);
+    free(buf);
     if (UNLIKELY(global_atom_index == ATOM_TABLE_NOT_FOUND)) {
         *error_reason = BADARG_ATOM;
         return term_invalid_term();

src/libAtomVM/bif.c

@@ -241,9 +241,8 @@ bool ets_hashtable_remove(struct EtsHashTable *hash_table, term key, size_t keyp
 
 static uint32_t hash_atom(term t, int32_t h, GlobalContext *global)
 {
-    AtomString atom_str = (uint8_t *) globalcontext_atomstring_from_term(global, t);
-    size_t len = atom_string_len(atom_str);
-    const uint8_t *data = (const uint8_t *) atom_string_data(atom_str);
+    size_t len;
+    const uint8_t *data = atom_table_get_atom_string(global->atom_table, term_to_atom_index(t), &len);
     for (size_t i = 0; i < len; ++i) {
         h = h * LARGE_PRIME_ATOM + data[i];
     }

src/libAtomVM/ets_hashtable.c

@@ -93,8 +93,8 @@ struct Nif
 struct UnresolvedFunctionCall
 {
     struct ExportedFunction base;
-    int module_atom_index;
-    int function_atom_index;
+    AtomIndex module_atom_index;
+    AtomIndex function_atom_index;
     int arity;
 };
 

src/libAtomVM/exportedfunction.h

@@ -570,22 +570,7 @@ term globalcontext_get_registered_name_process(GlobalContext *glb, int local_pro
 
 bool globalcontext_is_atom_index_equal_to_atom_string(GlobalContext *glb, AtomIndex atom_index_a, AtomString atom_string_b)
 {
-    AtomString atom_string_a;
-    atom_string_a = atom_table_get_atom_string(glb->atom_table, atom_index_a);
-    return atom_are_equals(atom_string_a, atom_string_b);
-}
-
-AtomString globalcontext_atomstring_from_term(GlobalContext *glb, term t)
-{
-    if (!term_is_atom(t)) {
-        AVM_ABORT();
-    }
-    unsigned long atom_index = term_to_atom_index(t);
-    AtomString str = atom_table_get_atom_string(glb->atom_table, atom_index);
-    if (IS_NULL_PTR(str)) {
-        return NULL;
-    }
-    return str;
+    return atom_table_is_equal_to_atom_string(glb->atom_table, atom_index_a, atom_string_b);
 }
 
 term globalcontext_existing_term_from_atom_string(GlobalContext *glb, AtomString atom_string)
@@ -662,28 +647,30 @@ Module *globalcontext_get_module(GlobalContext *global, AtomIndex module_name_at
     Module *found_module = (Module *) atomshashtable_get_value(global->modules_table, module_name_atom, (unsigned long) NULL);
 
     if (!found_module) {
-        char *module_name = malloc(256 + 5);
-        if (IS_NULL_PTR(module_name)) {
+        size_t module_name_len;
+        const uint8_t *module_name = atom_table_get_atom_string(global->atom_table, module_name_atom, &module_name_len);
+
+        char *module_file_name = malloc(module_name_len + 6);
+        if (IS_NULL_PTR(module_file_name)) {
             return NULL;
         }
-
-        atom_string_to_c(module_name_atom, module_name, 256);
-        strcat(module_name, ".beam");
-        Module *loaded_module = globalcontext_load_module_from_avm(global, module_name);
+        memcpy(module_file_name, module_name, module_name_len);
+        memcpy(module_file_name + module_name_len, ".beam", 6);
+        Module *loaded_module = globalcontext_load_module_from_avm(global, module_file_name);
         if (IS_NULL_PTR(loaded_module)) {
             // Platform may implement sys_load_module_from_file
-            loaded_module = sys_load_module_from_file(global, module_name);
+            loaded_module = sys_load_module_from_file(global, module_file_name);
         }
         if (UNLIKELY(!loaded_module || (globalcontext_insert_module(global, loaded_module) < 0))) {
-            fprintf(stderr, "Failed load module: %s\n", module_name);
-            free(module_name);
+            fprintf(stderr, "Failed load module: %s\n", module_file_name);
+            free(module_file_name);
             if (loaded_module) {
                 module_destroy(loaded_module);
             }
             return NULL;
         }
 
-        free(module_name);
+        free(module_file_name);
 
         return loaded_module;
     }

src/libAtomVM/globalcontext.c

@@ -357,22 +357,22 @@ void globalcontext_maybe_unregister_process_id(GlobalContext *glb, int process_i
  * assumes "ownership" of the allocated memory.
  * @returns newly added atom id or -1 in case of failure.
  */
-static inline int globalcontext_insert_atom_maybe_copy(GlobalContext *glb, AtomString atom_string, int copy)
+static inline int globalcontext_insert_atom_maybe_copy(GlobalContext *glb, const uint8_t *atom_data, size_t atom_len, int copy)
 {
-    long index = atom_table_ensure_atom(
-        glb->atom_table, atom_string, copy ? AtomTableCopyAtom : AtomTableNoOpts);
+    int index = atom_table_ensure_atom(
+        glb->atom_table, atom_data, atom_len, copy ? AtomTableCopyAtom : AtomTableNoOpts);
     if (UNLIKELY(index) < 0) {
         return -1;
     }
     return index;
 }
 
 /**
- * @brief equivalent to globalcontext_insert_atom_maybe_copy(glb, atom_string, 0);
+ * @brief equivalent to globalcontext_insert_atom_maybe_copy(glb, atom_string_data(atom_string), atom_string_len(atom_string), 0);
  */
 static inline int globalcontext_insert_atom(GlobalContext *glb, AtomString atom_string)
 {
-    return globalcontext_insert_atom_maybe_copy(glb, atom_string, 0);
+    return globalcontext_insert_atom_maybe_copy(glb, atom_string_data(atom_string), atom_string_len(atom_string), 0);
 }
 
 /**
@@ -420,20 +420,6 @@ static inline term globalcontext_make_atom(GlobalContext *glb, AtomString string
     return term_from_atom_index(global_atom_index);
 }
 
-/**
- * @brief   Returns the AtomString value of a term.
- *
- * @details This function fetches the AtomString value of the atom associated
- *          with the supplied term.  The input term must be an atom type.
- *          If no such atom is registered in the global table, this function
- *          returns NULL.  The caller should NOT free the data associated with
- *          the returned value.
- * @param   glb the global context
- * @param   t the atom term
- * @returns the AtomString associated with the supplied atom term.
- */
-AtomString globalcontext_atomstring_from_term(GlobalContext *glb, term t);
-
 /**
  * @brief Returns the term for an existing atom.
  *

src/libAtomVM/globalcontext.h

@@ -197,19 +197,7 @@ char *interop_list_to_utf8_string(term list, int *ok)
 char *interop_atom_to_string(Context *ctx, term atom)
 {
     GlobalContext *glb = ctx->global;
-
-    int atom_index = term_to_atom_index(atom);
-
-    size_t len;
-    atom_ref_t atom_ref = atom_table_get_atom_ptr_and_len(glb->atom_table, atom_index, &len);
-
-    char *str = malloc(len + 1);
-    if (IS_NULL_PTR(str)) {
-        return NULL;
-    }
-    atom_table_write_cstring(glb->atom_table, atom_ref, len + 1, str);
-
-    return str;
+    return atom_table_atom_to_new_cstring(glb->atom_table, term_to_atom_index(atom));
 }
 
 term interop_proplist_get_value(term list, term key)