Skip to content

Commit

Permalink
refactor: don't hide pointer in beam.Env declaration
Browse files Browse the repository at this point in the history
NASA's Power of Ten, Rule 9
[...] Pointer dereference operations may not be hidden in macro
definitions or inside typedef declarations.

By explicitly passing beam.Env as a pointer, we also are able to assert
non-nullability once at the source (when receiving, allocating the env, or
casting it from an int) and then pass around a pointer that is guaranteed to be
non-null.

A similar change for beam.ResourceType will be done when refactoring code around
NIF resources.
  • Loading branch information
rbino committed Mar 8, 2025
1 parent 93ed78c commit d87fa96
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 53 deletions.
46 changes: 23 additions & 23 deletions src/beam.zig
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub const allocator = @import("beam/allocator.zig");
pub const nif = @import("beam/nif.zig");
pub const resource = @import("beam/resource.zig");

pub const Env = ?*e.ErlNifEnv;
pub const Env = e.ErlNifEnv;
pub const Pid = e.ErlNifPid;
pub const ResourceType = ?*e.ErlNifResourceType;
pub const Term = e.ERL_NIF_TERM;
Expand All @@ -26,70 +26,70 @@ pub const large_allocator = allocator.large_allocator;
pub const general_purpose_allocator = allocator.general_purpose_allocator;

/// Raises a `:badarg` exception
pub fn raise_badarg(env: Env) Term {
pub fn raise_badarg(env: *Env) Term {
return e.enif_make_badarg(env);
}

/// Creates a ref
pub fn make_ref(env: Env) Term {
pub fn make_ref(env: *Env) Term {
return e.enif_make_ref(env);
}

/// Creates an atom from a Zig char slice
pub fn make_atom(env: Env, atom_str: []const u8) Term {
pub fn make_atom(env: *Env, atom_str: []const u8) Term {
return e.enif_make_atom_len(env, atom_str.ptr, atom_str.len);
}

/// Creates a beam `nil` atom
pub fn make_nil(env: Env) Term {
pub fn make_nil(env: *Env) Term {
return e.enif_make_atom(env, "nil");
}

/// Creates a beam `:ok` atom
pub fn make_ok(env: Env) Term {
pub fn make_ok(env: *Env) Term {
return e.enif_make_atom(env, "ok");
}

/// Helper to create an `{:ok, term}` tuple
pub fn make_ok_term(env: Env, val: Term) Term {
pub fn make_ok_term(env: *Env, val: Term) Term {
return e.enif_make_tuple(env, 2, make_ok(env), val);
}

/// Creates a beam `:error` atom.
pub fn make_error(env: Env) Term {
pub fn make_error(env: *Env) Term {
return e.enif_make_atom(env, "error");
}

/// Helper to create an `{:error, term}` tuple
pub fn make_error_term(env: Env, val: Term) Term {
pub fn make_error_term(env: *Env, val: Term) Term {
return e.enif_make_tuple(env, 2, make_error(env), val);
}

/// Helper to create an `{:error, atom}` tuple, taking the atom value from a slice
pub fn make_error_atom(env: Env, atom_str: []const u8) Term {
pub fn make_error_atom(env: *Env, atom_str: []const u8) Term {
return make_error_term(env, make_atom(env, atom_str));
}

/// Creates a binary term from a Zig slice
pub fn make_slice(env: Env, val: []const u8) Term {
pub fn make_slice(env: *Env, val: []const u8) Term {
var result: Term = undefined;
var bin: [*]u8 = @ptrCast(e.enif_make_new_binary(env, val.len, &result));
@memcpy(bin[0..val.len], val);

return result;
}

pub fn make_copy(destination_env: Env, source_term: Term) Term {
pub fn make_copy(destination_env: *Env, source_term: Term) Term {
return e.enif_make_copy(destination_env, source_term);
}

/// Creates a u8 value term.
pub fn make_u8(env: Env, val: u8) Term {
pub fn make_u8(env: *Env, val: u8) Term {
return e.enif_make_uint(env, val);
}

/// Creates an BEAM tuple from a Zig tuple of terms
pub fn make_tuple(env: Env, tuple: anytype) Term {
pub fn make_tuple(env: *Env, tuple: anytype) Term {
const type_info = @typeInfo(@TypeOf(tuple));
if (type_info != .Struct or !type_info.Struct.is_tuple)
@compileError("invalid argument to make_tuple: not a tuple");
Expand All @@ -105,7 +105,7 @@ pub fn make_tuple(env: Env, tuple: anytype) Term {
pub const GetError = error{ArgumentError};

/// Extract a binary from a term, returning it as a slice
pub fn get_char_slice(env: Env, src_term: Term) GetError![]u8 {
pub fn get_char_slice(env: *Env, src_term: Term) GetError![]u8 {
var bin: e.ErlNifBinary = undefined;
if (e.enif_inspect_binary(env, src_term, &bin) == 0) {
return GetError.ArgumentError;
Expand All @@ -115,7 +115,7 @@ pub fn get_char_slice(env: Env, src_term: Term) GetError![]u8 {
}

/// Extract a u128 from a binary (little endian) term
pub fn get_u128(env: Env, src_term: Term) GetError!u128 {
pub fn get_u128(env: *Env, src_term: Term) GetError!u128 {
const bin = try get_char_slice(env, src_term);
const required_length = @sizeOf(u128) / @sizeOf(u8);

Expand All @@ -126,25 +126,25 @@ pub fn get_u128(env: Env, src_term: Term) GetError!u128 {
}

/// Allocates a process independent environment
pub fn alloc_env() Env {
pub fn alloc_env() *Env {
const env = e.enif_alloc_env();
assert(env != null);
return env;
// Assert that the allocated environment is not null
return env.?;
}

/// Clears a process independent environment
pub fn clear_env(env: Env) void {
pub fn clear_env(env: *Env) void {
e.enif_clear_env(env);
}

/// Frees a process independent environment
pub fn free_env(env: Env) void {
pub fn free_env(env: *Env) void {
e.enif_free_env(env);
}

pub const SelfError = error{NotProcessBound};

pub fn self(env: Env) SelfError!Pid {
pub fn self(env: *Env) SelfError!Pid {
var result: Pid = undefined;
if (e.enif_self(env, &result) == null) {
return error.NotProcessBound;
Expand All @@ -155,7 +155,7 @@ pub fn self(env: Env) SelfError!Pid {

pub const SendError = error{NotDelivered};

pub fn send(dest: Pid, msg_env: Env, msg: Term) SendError!void {
pub fn send(dest: Pid, msg_env: *Env, msg: Term) SendError!void {
// Needed since enif_send is not const-correct
var to_pid = dest;

Expand Down
15 changes: 8 additions & 7 deletions src/beam/nif.zig
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ const e = @import("erl_nif.zig");
const beam = @import("../beam.zig");
const resource = @import("resource.zig");

pub const Nif = *const fn (beam.Env, argc: c_int, argv: [*c]const beam.Term) callconv(.C) beam.Term;
pub const NifLoadFn = *const fn (beam.Env, [*c]?*anyopaque, beam.Term) callconv(.C) c_int;
pub const Nif = *const fn (?*beam.Env, argc: c_int, argv: [*c]const beam.Term) callconv(.C) beam.Term;
pub const NifLoadFn = *const fn (?*beam.Env, [*c]?*anyopaque, beam.Term) callconv(.C) c_int;

pub const FunctionEntry = e.ErlNifFunc;
pub const Entrypoint = e.ErlNifEntry;
Expand Down Expand Up @@ -58,16 +58,16 @@ fn MakeWrappedNif(comptime fun: anytype) type {
const ReturnType = function_info.return_type.?;
comptime assert(ReturnType == beam.Term);

// And since we need to construct a beam.Term, we always accept a beam.Env as first parameter
// And since we need to construct a beam.Term, we always accept a BEAM env as first parameter
const params = function_info.params;
comptime assert(params[0].type == beam.Env);
comptime assert(params[0].type == *beam.Env);

return struct {
// Env is not counted towards the effective arity, subtract 1 since env is the first parameter
pub const arity = params.len - 1;

pub fn wrapper(
env: beam.Env,
env: ?*beam.Env,
argc: c_int,
argv_ptr: [*c]const beam.Term,
) callconv(.C) beam.Term {
Expand All @@ -82,8 +82,9 @@ fn MakeWrappedNif(comptime fun: anytype) type {
var args: std.meta.ArgsTuple(Function) = undefined;
inline for (&args, 0..) |*arg, i| {
if (i == 0) {
// Put the env as first argument
arg.* = env;
// Put the env as first argument, asserting it's not null so we change
// the type from ?*beam.Env to *beam.Env
arg.* = env.?;
} else {
// Check that the function accepts only beam.Term arguments
const ArgType = @TypeOf(arg.*);
Expand Down
10 changes: 5 additions & 5 deletions src/beam/resource.zig
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub const Error = error{
OutOfMemory,
};

const DeinitFn = *const fn (beam.Env, ?*anyopaque) callconv(.C) void;
const DeinitFn = *const fn (?*beam.Env, ?*anyopaque) callconv(.C) void;

pub fn Resource(comptime T: anytype, comptime deinit_fn: ?DeinitFn) type {
return struct {
Expand All @@ -18,7 +18,7 @@ pub fn Resource(comptime T: anytype, comptime deinit_fn: ?DeinitFn) type {
const Type = struct {
beam_type: beam.ResourceType,

pub fn open(env: beam.Env, name: [:0]const u8) Type {
pub fn open(env: *beam.Env, name: [:0]const u8) Type {
const beam_type = e.enif_open_resource_type(
env,
null,
Expand All @@ -40,7 +40,7 @@ pub fn Resource(comptime T: anytype, comptime deinit_fn: ?DeinitFn) type {

/// Initializes the type of the resource. This must be called exactly once
/// in the load or upgrade callback of the NIF.
pub fn create_type(env: beam.Env, name: [:0]const u8) void {
pub fn create_type(env: *beam.Env, name: [:0]const u8) void {
// TODO: is this required or are we allowed to re-open a type?
assert(resource_type == null);

Expand Down Expand Up @@ -69,7 +69,7 @@ pub fn Resource(comptime T: anytype, comptime deinit_fn: ?DeinitFn) type {
}

/// Recreates the resource from the term handle obtained with `term_handle`
pub fn from_term_handle(env: beam.Env, term: beam.Term) !Self {
pub fn from_term_handle(env: *beam.Env, term: beam.Term) !Self {
var raw_ptr: ?*anyopaque = undefined;

if (0 == e.enif_get_resource(env, term, res_type(), &raw_ptr)) {
Expand All @@ -80,7 +80,7 @@ pub fn Resource(comptime T: anytype, comptime deinit_fn: ?DeinitFn) type {
}

/// Obtains a term handle to the resource
pub fn term_handle(self: Self, env: beam.Env) beam.Term {
pub fn term_handle(self: Self, env: *beam.Env) beam.Term {
return e.enif_make_resource(env, self.raw_ptr);
}

Expand Down
32 changes: 16 additions & 16 deletions src/client.zig
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const InitClientError = error{
Unexpected,
};

pub fn init(env: beam.Env, cluster_id_term: beam.Term, addresses_term: beam.Term) beam.Term {
pub fn init(env: *beam.Env, cluster_id_term: beam.Term, addresses_term: beam.Term) beam.Term {
return init_client(env, cluster_id_term, addresses_term) catch |err| switch (err) {
error.AddressInvalid => beam.make_error_atom(env, "invalid_address"),
error.AddressLimitExceeded => beam.make_error_atom(env, "address_limit_exceeded"),
Expand All @@ -42,7 +42,7 @@ pub fn init(env: beam.Env, cluster_id_term: beam.Term, addresses_term: beam.Term
};
}

fn init_client(env: beam.Env, cluster_id_term: beam.Term, addresses_term: beam.Term) InitClientError!beam.Term {
fn init_client(env: *beam.Env, cluster_id_term: beam.Term, addresses_term: beam.Term) InitClientError!beam.Term {
const cluster_id = try beam.get_u128(env, cluster_id_term);
const addresses = try beam.get_char_slice(env, addresses_term);

Expand Down Expand Up @@ -76,7 +76,7 @@ const SubmitError = error{
ArgumentError,
};

pub fn create_accounts(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn create_accounts(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -85,7 +85,7 @@ pub fn create_accounts(env: beam.Env, client_resource: beam.Term, payload_term:
) catch |err| handle_submit_error(env, err);
}

pub fn create_transfers(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn create_transfers(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -94,7 +94,7 @@ pub fn create_transfers(env: beam.Env, client_resource: beam.Term, payload_term:
) catch |err| handle_submit_error(env, err);
}

pub fn lookup_accounts(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn lookup_accounts(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -103,7 +103,7 @@ pub fn lookup_accounts(env: beam.Env, client_resource: beam.Term, payload_term:
) catch |err| handle_submit_error(env, err);
}

pub fn lookup_transfers(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn lookup_transfers(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -112,7 +112,7 @@ pub fn lookup_transfers(env: beam.Env, client_resource: beam.Term, payload_term:
) catch |err| handle_submit_error(env, err);
}

pub fn get_account_transfers(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn get_account_transfers(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -121,7 +121,7 @@ pub fn get_account_transfers(env: beam.Env, client_resource: beam.Term, payload_
) catch |err| handle_submit_error(env, err);
}

pub fn get_account_balances(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn get_account_balances(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -130,7 +130,7 @@ pub fn get_account_balances(env: beam.Env, client_resource: beam.Term, payload_t
) catch |err| handle_submit_error(env, err);
}

pub fn query_accounts(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn query_accounts(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -139,7 +139,7 @@ pub fn query_accounts(env: beam.Env, client_resource: beam.Term, payload_term: b
) catch |err| handle_submit_error(env, err);
}

pub fn query_transfers(env: beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
pub fn query_transfers(env: *beam.Env, client_resource: beam.Term, payload_term: beam.Term) beam.Term {
return submit(
env,
client_resource,
Expand All @@ -149,7 +149,7 @@ pub fn query_transfers(env: beam.Env, client_resource: beam.Term, payload_term:
}

fn submit(
env: beam.Env,
env: *beam.Env,
client_term: beam.Term,
operation: tb_client.Operation,
payload_term: beam.Term,
Expand All @@ -173,7 +173,7 @@ fn submit(

// Retrieve the process independent environment that is stored in the completion context
const completion_ctx = try client.completion_context();
const tigerbeetle_env: beam.Env = @ptrFromInt(completion_ctx);
const tigerbeetle_env: *beam.Env = @ptrFromInt(completion_ctx);

// Copy over the ref and the payload term in the process independent environment
// Those need to be accessible in the on_completion callback, so we must copy them over
Expand Down Expand Up @@ -212,7 +212,7 @@ fn submit(
return beam.make_ok_term(env, ref);
}

fn handle_submit_error(env: beam.Env, err: SubmitError) beam.Term {
fn handle_submit_error(env: *beam.Env, err: SubmitError) beam.Term {
return switch (err) {
error.InvalidResourceTerm => beam.make_error_atom(env, "invalid_client_resource"),
error.TooManyRequests => beam.make_error_atom(env, "too_many_requests"),
Expand All @@ -237,7 +237,7 @@ fn on_completion(
// Decrease client refcount after we exit
defer resource.raw_release(ctx.client_raw_obj);

const env: beam.Env = @ptrFromInt(context);
const env: *beam.Env = @ptrFromInt(context);
defer beam.clear_env(env);

const ref = ctx.request_ref;
Expand Down Expand Up @@ -272,7 +272,7 @@ fn on_completion(
beam.send(caller_pid, env, msg) catch {};
}

fn client_resource_deinit_fn(_: beam.Env, ptr: ?*anyopaque) callconv(.C) void {
fn client_resource_deinit_fn(_: ?*beam.Env, ptr: ?*anyopaque) callconv(.C) void {
if (ptr) |p| {
const client: *ClientInterface = @ptrCast(@alignCast(p));
// The client was already closed, we just return
Expand All @@ -282,7 +282,7 @@ fn client_resource_deinit_fn(_: beam.Env, ptr: ?*anyopaque) callconv(.C) void {
// The client was already closed, we just return
error.ClientInvalid => return,
};
const env: beam.Env = @ptrFromInt(completion_ctx);
const env: *beam.Env = @ptrFromInt(completion_ctx);
beam.free_env(env);
} else unreachable;
}
4 changes: 2 additions & 2 deletions src/tigerbeetlex.zig
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ var exported_nifs = [_]nif.FunctionEntry{
nif.wrap("query_transfers", client.query_transfers),
};

fn nif_load(env: beam.Env, _: [*c]?*anyopaque, _: beam.Term) callconv(.C) c_int {
ClientResource.create_type(env, "TigerBeetlex.Client");
fn nif_load(env: ?*beam.Env, _: [*c]?*anyopaque, _: beam.Term) callconv(.C) c_int {
ClientResource.create_type(env.?, "TigerBeetlex.Client");
return 0;
}

Expand Down

0 comments on commit d87fa96

Please sign in to comment.