Back to all reviewers

Validate nullability explicitly

oven-sh/bun
Based on 7 comments
Other

Always validate null or undefined values before using them to prevent crashes and undefined behavior. When implementing null-handling code, consider these principles:

Null Handling Other

Reviewer Prompt

Always validate null or undefined values before using them to prevent crashes and undefined behavior. When implementing null-handling code, consider these principles:

  1. Use nullable types (like ?*Type in Zig) when a value might be null, rather than assuming non-nullability:
// BAD - Compiler will optimize out this check
fn requestTermination(handle: *WebWorkerLifecycleHandle) void {
    if (@intFromPtr(handle) == 0) return;
    // ...
}

// GOOD - Explicitly mark as nullable
fn requestTermination(handle: ?*WebWorkerLifecycleHandle) void {
    if (handle == null) return;
    // ...
}
  1. Never default-initialize struct fields with undefined unless they’re guaranteed to be assigned before use:
// BAD - Creates potential undefined behavior
struct {
    last_modified_buffer: [32]u8 = undefined,
}

// GOOD - Safe initialization
struct {
    last_modified_buffer: [32]u8 = [_]u8{0} ** 32,
}
  1. Always initialize boolean flags and state fields in constructors:
// BAD - Uninitialized fields
return bun.new(FileRoute, .{
    .ref_count = .init(),
    .server = opts.server,
    .blob = blob,
    .headers = headers,
    .status_code = opts.status_code,
    // Missing has_last_modified_header initialization
});

// GOOD - All fields explicitly initialized
return bun.new(FileRoute, .{
    .ref_count = .init(),
    .server = opts.server,
    .blob = blob,
    .headers = headers,
    .status_code = opts.status_code,
    .has_last_modified_header = headers.get("last-modified") != null,
    .has_content_length_header = headers.get("content-length") != null,
});
  1. Respect API contracts regarding null handling. When wrapping libraries like V8, understand their null semantics:
// DANGEROUS - Will crash if MaybeLocal is empty
Local<T> ToLocalChecked() const {
    return m_local;
}

// SAFE - Explicit check before dereferencing
bool ToLocal(Local<T>* out) const {
    if (IsEmpty()) {
        return false;
    }
    *out = m_local;
    return true;
}
7
Comments Analyzed
Other
Primary Language
Null Handling
Category

Source Discussions