Skip to content

Commit f911fba

Browse files
jgonetjgonet
committed
ets: combine table access and permission check
This code was repeated in all of the places and was creating opportunities for copy & paste errors. Signed-off-by: Jakub Gonet <jakub.gonet@swmansion.com>
1 parent 1616e62 commit f911fba

File tree

3 files changed

+61
-113
lines changed

3 files changed

+61
-113
lines changed

‎src/libAtomVM/ets.c

Lines changed: 53 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
#include "overflow_helpers.h"
2929
#include "term.h"
3030

31+
#define ETS_NO_INDEX SIZE_MAX
32+
#define ETS_ANY_PROCESS -1
33+
3134
#ifndef AVM_NO_SMP
3235
#define SMP_RDLOCK(table) smp_rwlock_rdlock(table->lock)
3336
#define SMP_WRLOCK(table) smp_rwlock_wrlock(table->lock)
@@ -81,51 +84,40 @@ static void ets_add_table(struct Ets *ets, struct EtsTable *ets_table)
8184
synclist_unlock(&ets->ets_tables);
8285
}
8386

84-
static struct EtsTable *ets_get_table_by_ref(struct Ets *ets, uint64_t ref, TableAccessType access_type)
87+
static struct EtsTable *ets_acquire_table(struct Ets *ets, int32_t process_id, term name_or_ref, TableAccessType requested_access_type)
8588
{
86-
struct ListHead *ets_tables_list = synclist_rdlock(&ets->ets_tables);
87-
struct ListHead *item;
88-
struct EtsTable *ret = NULL;
89-
LIST_FOR_EACH (item, ets_tables_list) {
90-
struct EtsTable *table = GET_LIST_ENTRY(item, struct EtsTable, head);
91-
if (table->ref_ticks == ref) {
92-
switch (access_type) {
93-
case TableAccessRead:
94-
SMP_RDLOCK(table);
95-
break;
96-
case TableAccessWrite:
97-
SMP_WRLOCK(table);
98-
break;
99-
default:
100-
break;
101-
}
102-
ret = table;
103-
break;
104-
}
89+
uint64_t ref = 0;
90+
term name = term_invalid_term();
91+
bool is_atom = term_is_atom(name_or_ref);
92+
if (is_atom) {
93+
name = name_or_ref;
94+
} else {
95+
ref = term_to_ref_ticks(name_or_ref);
10596
}
106-
synclist_unlock(&ets->ets_tables);
107-
return ret;
108-
}
10997

110-
static struct EtsTable *ets_get_table_by_name(struct Ets *ets, term name, TableAccessType access_type)
111-
{
98+
struct EtsTable *ret = NULL;
11299
struct ListHead *ets_tables_list = synclist_rdlock(&ets->ets_tables);
113100
struct ListHead *item;
114-
struct EtsTable *ret = NULL;
115101
LIST_FOR_EACH (item, ets_tables_list) {
116102
struct EtsTable *table = GET_LIST_ENTRY(item, struct EtsTable, head);
117-
if (table->is_named && table->name == name) {
118-
switch (access_type) {
119-
case TableAccessRead:
120-
SMP_RDLOCK(table);
121-
break;
122-
case TableAccessWrite:
123-
SMP_WRLOCK(table);
124-
break;
125-
default:
126-
break;
103+
bool found = is_atom ? table->is_named && table->name == name : table->ref_ticks == ref;
104+
if (found) {
105+
bool is_owner = table->owner_process_id == process_id;
106+
bool is_non_private = table->access_type != EtsAccessPrivate;
107+
bool is_public = table->access_type == EtsAccessPublic;
108+
109+
bool can_read = requested_access_type == TableAccessRead && (is_non_private || is_owner);
110+
bool can_write = requested_access_type == TableAccessWrite && (is_public || is_owner);
111+
bool access_none = requested_access_type == TableAccessNone;
112+
if (can_read) {
113+
SMP_RDLOCK(table);
114+
ret = table;
115+
} else if (can_write) {
116+
SMP_WRLOCK(table);
117+
ret = table;
118+
} else if (access_none) {
119+
ret = table;
127120
}
128-
ret = table;
129121
break;
130122
}
131123
}
@@ -147,7 +139,7 @@ void ets_destroy(struct Ets *ets, GlobalContext *global)
147139
EtsErrorCode ets_create_table_maybe_gc(term name, bool is_named, EtsTableType table_type, EtsAccessType access_type, size_t keypos, term *ret, Context *ctx)
148140
{
149141
if (is_named) {
150-
struct EtsTable *ets_table = ets_get_table_by_name(&ctx->global->ets, name, TableAccessNone);
142+
struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ETS_ANY_PROCESS, name, TableAccessNone);
151143
if (ets_table != NULL) {
152144
return EtsTableNameInUse;
153145
}
@@ -255,10 +247,6 @@ static void ets_delete_all_tables(struct Ets *ets, GlobalContext *global)
255247

256248
static EtsErrorCode ets_table_insert(struct EtsTable *ets_table, term entry, Context *ctx)
257249
{
258-
if (ets_table->access_type != EtsAccessPublic && ets_table->owner_process_id != ctx->process_id) {
259-
return EtsPermissionDenied;
260-
}
261-
262250
size_t keypos = ets_table->keypos;
263251

264252
if ((size_t) term_get_tuple_arity(entry) < keypos + 1) {
@@ -270,7 +258,7 @@ static EtsErrorCode ets_table_insert(struct EtsTable *ets_table, term entry, Con
270258
return EtsAllocationFailure;
271259
}
272260

273-
EtsHashtableErrorCode res = ets_hashtable_insert(ets_table->hashtable, new_node, EtsHashtableAllowOverwrite, ctx->global);
261+
EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, new_node, EtsHashtableAllowOverwrite, ctx->global);
274262
if (UNLIKELY(res != EtsHashtableOk)) {
275263
return EtsAllocationFailure;
276264
}
@@ -280,10 +268,6 @@ static EtsErrorCode ets_table_insert(struct EtsTable *ets_table, term entry, Con
280268

281269
static EtsErrorCode ets_table_insert_list(struct EtsTable *ets_table, term list, Context *ctx)
282270
{
283-
if (ets_table->access_type != EtsAccessPublic && ets_table->owner_process_id != ctx->process_id) {
284-
return EtsPermissionDenied;
285-
}
286-
287271
term iter = list;
288272
size_t size = 0;
289273

@@ -318,8 +302,7 @@ static EtsErrorCode ets_table_insert_list(struct EtsTable *ets_table, term list,
318302
}
319303

320304
for (size_t i = 0; i < size; ++i) {
321-
322-
EtsHashtableErrorCode res = ets_hashtable_insert(ets_table->hashtable, nodes[i], EtsHashtableAllowOverwrite, ctx->global);
305+
EtsHashtableStatus res = ets_hashtable_insert(ets_table->hashtable, nodes[i], EtsHashtableAllowOverwrite, ctx->global);
323306
assert(res == EtsHashtableOk);
324307
}
325308

@@ -329,9 +312,9 @@ static EtsErrorCode ets_table_insert_list(struct EtsTable *ets_table, term list,
329312

330313
EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx)
331314
{
332-
struct EtsTable *ets_table = term_is_atom(name_or_ref) ? ets_get_table_by_name(&ctx->global->ets, name_or_ref, TableAccessWrite) : ets_get_table_by_ref(&ctx->global->ets, term_to_ref_ticks(name_or_ref), TableAccessWrite);
333-
if (ets_table == NULL) {
334-
return EtsTableNotFound;
315+
struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite);
316+
if (IS_NULL_PTR(ets_table)) {
317+
return EtsBadAccess;
335318
}
336319

337320
EtsErrorCode result;
@@ -350,10 +333,6 @@ EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx)
350333

351334
static EtsErrorCode ets_table_lookup_maybe_gc(struct EtsTable *ets_table, term key, term *ret, Context *ctx, int num_roots, term *roots)
352335
{
353-
if (ets_table->access_type == EtsAccessPrivate && ets_table->owner_process_id != ctx->process_id) {
354-
return EtsPermissionDenied;
355-
}
356-
357336
term res = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->keypos, ctx->global);
358337

359338
if (term_is_nil(res)) {
@@ -374,9 +353,9 @@ static EtsErrorCode ets_table_lookup_maybe_gc(struct EtsTable *ets_table, term k
374353

375354
EtsErrorCode ets_lookup_maybe_gc(term name_or_ref, term key, term *ret, Context *ctx)
376355
{
377-
struct EtsTable *ets_table = term_is_atom(name_or_ref) ? ets_get_table_by_name(&ctx->global->ets, name_or_ref, TableAccessRead) : ets_get_table_by_ref(&ctx->global->ets, term_to_ref_ticks(name_or_ref), TableAccessRead);
378-
if (ets_table == NULL) {
379-
return EtsTableNotFound;
356+
struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessRead);
357+
if (IS_NULL_PTR(ets_table)) {
358+
return EtsBadAccess;
380359
}
381360

382361
EtsErrorCode result = ets_table_lookup_maybe_gc(ets_table, key, ret, ctx, 0, NULL);
@@ -391,14 +370,9 @@ EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t pos,
391370
return EtsBadPosition;
392371
}
393372

394-
struct EtsTable *ets_table = term_is_atom(name_or_ref) ? ets_get_table_by_name(&ctx->global->ets, name_or_ref, TableAccessRead) : ets_get_table_by_ref(&ctx->global->ets, term_to_ref_ticks(name_or_ref), TableAccessRead);
395-
if (ets_table == NULL) {
396-
return EtsTableNotFound;
397-
}
398-
399-
if (ets_table->access_type == EtsAccessPrivate && ets_table->owner_process_id != ctx->process_id) {
400-
SMP_UNLOCK(ets_table);
401-
return EtsPermissionDenied;
373+
struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessRead);
374+
if (IS_NULL_PTR(ets_table)) {
375+
return EtsBadAccess;
402376
}
403377

404378
term entry = ets_hashtable_lookup(ets_table->hashtable, key, ets_table->keypos, ctx->global);
@@ -426,29 +400,11 @@ EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t pos,
426400
return EtsOk;
427401
}
428402

429-
static EtsErrorCode ets_table_delete(struct EtsTable *ets_table, term key, term *ret, Context *ctx)
430-
{
431-
if (ets_table->access_type != EtsAccessPublic && ets_table->owner_process_id != ctx->process_id) {
432-
return EtsPermissionDenied;
433-
}
434-
435-
bool _res = ets_hashtable_remove(ets_table->hashtable, key, ets_table->keypos, ctx->global);
436-
UNUSED(_res);
437-
438-
*ret = TRUE_ATOM;
439-
return EtsOk;
440-
}
441-
442403
EtsErrorCode ets_drop_table(term name_or_ref, term *ret, Context *ctx)
443404
{
444-
struct EtsTable *ets_table = term_is_atom(name_or_ref)
445-
? ets_get_table_by_name(&ctx->global->ets, name_or_ref, TableAccessWrite)
446-
: ets_get_table_by_ref(&ctx->global->ets, term_to_ref_ticks(name_or_ref), TableAccessWrite);
405+
struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite);
447406
if (IS_NULL_PTR(ets_table)) {
448-
return EtsTableNotFound;
449-
}
450-
if (ets_table->access_type != EtsAccessPublic && ets_table->owner_process_id != ctx->process_id) {
451-
return EtsPermissionDenied;
407+
return EtsBadAccess;
452408
}
453409

454410
struct ListHead *ets_tables_list = synclist_wrlock(&ctx->global->ets.ets_tables);
@@ -464,17 +420,17 @@ EtsErrorCode ets_drop_table(term name_or_ref, term *ret, Context *ctx)
464420

465421
EtsErrorCode ets_delete(term name_or_ref, term key, term *ret, Context *ctx)
466422
{
467-
struct EtsTable *ets_table = term_is_atom(name_or_ref)
468-
? ets_get_table_by_name(&ctx->global->ets, name_or_ref, TableAccessRead)
469-
: ets_get_table_by_ref(&ctx->global->ets, term_to_ref_ticks(name_or_ref), TableAccessRead);
423+
struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, name_or_ref, TableAccessWrite);
470424
if (IS_NULL_PTR(ets_table)) {
471-
return EtsTableNotFound;
425+
return EtsBadAccess;
472426
}
473427

474-
EtsErrorCode res = ets_table_delete(ets_table, key, ret, ctx);
475-
428+
bool _found = ets_hashtable_remove(ets_table->hashtable, key, ets_table->keypos, ctx->global);
429+
UNUSED(_found);
476430
SMP_UNLOCK(ets_table);
477-
return res;
431+
432+
*ret = TRUE_ATOM;
433+
return EtsOk;
478434
}
479435

480436
static bool operation_to_tuple4(term operation, size_t default_pos, term *position, term *increment, term *threshold, term *set_value)
@@ -524,11 +480,9 @@ static bool operation_to_tuple4(term operation, size_t default_pos, term *positi
524480

525481
EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, term default_value, term *ret, Context *ctx)
526482
{
527-
struct EtsTable *ets_table = term_is_atom(ref)
528-
? ets_get_table_by_name(&ctx->global->ets, ref, TableAccessWrite)
529-
: ets_get_table_by_ref(&ctx->global->ets, term_to_ref_ticks(ref), TableAccessWrite);
483+
struct EtsTable *ets_table = ets_acquire_table(&ctx->global->ets, ctx->process_id, ref, TableAccessWrite);
530484
if (IS_NULL_PTR(ets_table)) {
531-
return EtsTableNotFound;
485+
return EtsBadAccess;
532486
}
533487

534488
// do not use an invalid term as a root
@@ -588,7 +542,7 @@ EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, ter
588542
avm_int_t elem_value;
589543
if (BUILTIN_ADD_OVERFLOW_INT(increment, term_to_int(elem), &elem_value)) {
590544
SMP_UNLOCK(ets_table);
591-
return EtsOverlfow;
545+
return EtsOverflow;
592546
}
593547
if (!term_is_invalid_term(threshold_term) && !term_is_invalid_term(set_value_term)) {
594548
avm_int_t threshold = term_to_int(threshold_term);

‎src/libAtomVM/ets.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,13 @@ typedef enum EtsAccessType
5151
typedef enum EtsErrorCode
5252
{
5353
EtsOk,
54-
EtsTableNotFound,
54+
EtsBadAccess,
5555
EtsTableNameInUse,
56-
EtsPermissionDenied,
5756
EtsBadEntry,
5857
EtsAllocationFailure,
5958
EtsEntryNotFound,
6059
EtsBadPosition,
61-
EtsOverlfow
60+
EtsOverflow
6261
} EtsErrorCode;
6362
struct Ets
6463
{

‎src/libAtomVM/nifs.c

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3356,9 +3356,8 @@ static term nif_ets_insert(Context *ctx, int argc, term argv[])
33563356
switch (result) {
33573357
case EtsOk:
33583358
return TRUE_ATOM;
3359-
case EtsTableNotFound:
3359+
case EtsBadAccess:
33603360
case EtsBadEntry:
3361-
case EtsPermissionDenied:
33623361
RAISE_ERROR(BADARG_ATOM);
33633362
case EtsAllocationFailure:
33643363
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
@@ -3381,8 +3380,7 @@ static term nif_ets_lookup(Context *ctx, int argc, term argv[])
33813380
switch (result) {
33823381
case EtsOk:
33833382
return ret;
3384-
case EtsTableNotFound:
3385-
case EtsPermissionDenied:
3383+
case EtsBadAccess:
33863384
RAISE_ERROR(BADARG_ATOM);
33873385
case EtsAllocationFailure:
33883386
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
@@ -3409,8 +3407,7 @@ static term nif_ets_lookup_element(Context *ctx, int argc, term argv[])
34093407
return ret;
34103408
case EtsEntryNotFound:
34113409
case EtsBadPosition:
3412-
case EtsTableNotFound:
3413-
case EtsPermissionDenied:
3410+
case EtsBadAccess:
34143411
RAISE_ERROR(BADARG_ATOM);
34153412
case EtsAllocationFailure:
34163413
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
@@ -3435,8 +3432,7 @@ static term nif_ets_delete(Context *ctx, int argc, term argv[])
34353432
switch (result) {
34363433
case EtsOk:
34373434
return ret;
3438-
case EtsTableNotFound:
3439-
case EtsPermissionDenied:
3435+
case EtsBadAccess:
34403436
RAISE_ERROR(BADARG_ATOM);
34413437
case EtsAllocationFailure:
34423438
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
@@ -3465,13 +3461,12 @@ static term nif_ets_update_counter(Context *ctx, int argc, term argv[])
34653461
switch (result) {
34663462
case EtsOk:
34673463
return ret;
3468-
case EtsTableNotFound:
3469-
case EtsPermissionDenied:
3464+
case EtsBadAccess:
34703465
case EtsBadEntry:
34713466
RAISE_ERROR(BADARG_ATOM);
34723467
case EtsAllocationFailure:
34733468
RAISE_ERROR(OUT_OF_MEMORY_ATOM);
3474-
case EtsOverlfow:
3469+
case EtsOverflow:
34753470
RAISE_ERROR(OVERFLOW_ATOM);
34763471
default:
34773472
UNREACHABLE();

0 commit comments

Comments
 (0)