C Code Review Checklist: Memory Safety and Systems Programming

Published on
Written byChristoffer Artmann
C Code Review Checklist: Memory Safety and Systems Programming

Buffer overflows corrupt memory and create security vulnerabilities. Memory leaks exhaust resources under load. Null pointer dereferences crash programs. Use-after-free creates unpredictable behavior. These issues compile without warnings and work in simple tests but fail catastrophically in production.

This checklist helps catch what matters: memory safety violations, undefined behavior, resource leaks, and unsafe pointer operations that create bugs or security vulnerabilities.

Quick Reference Checklist

Use this checklist as a quick reference during code review.

Memory Management

  • Every malloc/calloc/realloc has corresponding free
  • malloc return values checked for NULL before use
  • Memory freed exactly once (no double-free)
  • Pointers set to NULL after free to catch use-after-free
  • calloc preferred over malloc + memset for zero-initialization
  • realloc handles potential NULL return without leaking original
  • Memory aligned appropriately for type (natural alignment)
  • No arithmetic on void* pointers (undefined behavior)

Pointer Safety

  • Pointers checked for NULL before dereferencing
  • No dereferencing uninitialized pointers
  • Pointers not used after free (use-after-free)
  • Function parameters validated for NULL when required
  • Array bounds checked before pointer arithmetic
  • Casts justified and type-correct
  • const used for pointers that don't modify target
  • Pointer arithmetic stays within allocated bounds

Buffer Handling

  • strncpy, snprintf used instead of strcpy, sprintf
  • Buffer sizes calculated correctly (including null terminator)
  • sizeof used on correct type (array not pointer)
  • String operations null-terminate explicitly when needed
  • Input length validated before copying
  • Off-by-one errors avoided in loops and indexing
  • memcpy not used with overlapping buffers (use memmove)
  • Bounds checking before array/buffer access

Error Handling

  • Return codes checked for all library functions
  • errno checked for functions that set it
  • Error paths free resources and clean up state
  • Failed allocations don't leak previously allocated memory
  • File operations check for errors (NULL, -1, EOF)
  • Error messages include context (function, operation)
  • No ignoring return values from critical functions
  • Cleanup code handles partial initialization

Resource Management

  • File handles closed on all paths (success and error)
  • Resources not leaked when early returns occur
  • fclose, close return values checked
  • Temporary files deleted on error paths
  • Locks/mutexes released on all code paths
  • Signal handlers async-signal-safe
  • No resource exhaustion from repeated operations
  • Cleanup order reverses acquisition order

Undefined Behavior Prevention

  • Variables initialized before use
  • No signed integer overflow
  • No dereferencing null or invalid pointers
  • Shifts by appropriate amounts (not >= width)
  • No reading uninitialized memory
  • Strict aliasing rules followed
  • No sequence point violations
  • volatile used for hardware registers and signal handlers

Memory Management Fundamentals

Every allocation requires corresponding deallocation. When we see malloc, calloc, or realloc, we trace the code paths to verify free is called on all paths. Missing frees create memory leaks that accumulate over program execution, eventually exhausting memory.

Checking malloc return values prevents null pointer dereferences. The pattern ptr = malloc(size); if (ptr == NULL) { /* handle error */ } validates allocation succeeded before using the pointer. Production code without these checks fails when memory is exhausted or fragmented.

Double-free corrupts heap metadata and causes crashes or security vulnerabilities. When we see free(ptr), we verify that pointer isn't freed again. Setting pointers to NULL after freeing helps catch double-frees: free(ptr); ptr = NULL; makes subsequent frees safe (freeing NULL is defined as no-op).

realloc requires careful handling because it might return a new pointer. The pattern new_ptr = realloc(ptr, new_size); if (new_ptr == NULL) { /* original ptr still valid */ } preserves the original pointer when realloc fails. Code that assigns ptr = realloc(ptr, size) leaks memory if realloc returns NULL.

calloc zero-initializes memory while malloc leaves contents undefined. When zero-initialization is needed, calloc(count, size) is clearer and potentially faster than malloc followed by memset. When we see malloc + memset, calloc expresses the intent better.

Alignment matters for performance and correctness on some architectures. malloc returns suitably aligned memory for any type, but pointer arithmetic can create misaligned pointers. Accessing misaligned data causes slow performance or crashes on some CPUs. When we see pointer arithmetic creating structure pointers, alignment needs verification.

Pointer Safety and Validation

Null pointer dereferences are among the most common C bugs. Before any pointer dereference, we verify the pointer is checked for NULL. The pattern if (ptr != NULL) { *ptr = value; } prevents crashes. Function parameters that might be NULL require checking at function entry.

Uninitialized pointers contain garbage values that appear to work but cause random crashes. Declarations should initialize pointers: int *ptr = NULL; or assign valid addresses before use. When we see pointer declarations without initialization, that's a potential bug.

Use-after-free creates security vulnerabilities and unpredictable behavior. Accessing memory after free might work initially if the memory hasn't been reused, but fails randomly when allocator reuses that memory. We verify pointers aren't used after the pointed-to memory is freed.

Pointer arithmetic must stay within bounds of allocated memory. The expression ptr + offset is only valid when the result points within the allocation or one past the end. Pointer arithmetic beyond these bounds causes undefined behavior even if we don't dereference the result.

Casts between incompatible pointer types violate strict aliasing rules and cause undefined behavior. The pattern (int*)float_ptr creates problems because the compiler assumes pointers to different types don't alias. When we see pointer casts, we verify they're necessary and correct.

const correctness documents which pointers modify their targets. The declaration const char *str indicates str points to immutable data. This enables compiler optimizations and prevents accidental modification. When pointers don't modify their targets, const makes this explicit.

Buffer Safety and String Handling

strcpy and sprintf are dangerous because they don't check buffer sizes. These functions continue copying until null terminator or format string completes, potentially overflowing buffers. Modern code uses strncpy, snprintf which accept buffer sizes. When we see the unsafe functions, that's a security vulnerability.

Buffer size calculations must account for null terminators. The pattern char buf[100]; snprintf(buf, sizeof(buf), ...) correctly sizes the buffer. Common mistakes include sizeof(buf) - 1 which wastes a byte, or using strlen on uninitialized buffers.

sizeof returns different values for arrays vs pointers. When a function receives an array parameter, it decays to a pointer, so sizeof(param) returns pointer size, not array size. Functions should take explicit size parameters: void process(char *buf, size_t size). When we see sizeof on function parameters, that's likely a bug.

String operations might not null-terminate buffers. strncpy(buf, src, size) doesn't guarantee null termination if src is >= size characters. Code using strncpy should explicitly null-terminate: buf[size-1] = '\0'. When we see strncpy without explicit termination, that's a potential bug.

Off-by-one errors in loops and indexing cause buffer overflows. The loop for (i = 0; i <= size; i++) accesses one past the end. Correct bounds are i < size. When reviewing loops that access arrays, we verify bounds carefully.

memcpy requires non-overlapping source and destination. When buffers might overlap, memmove handles this correctly. Using memcpy with overlapping buffers causes undefined behavior. We verify memcpy calls don't overlap or should use memmove.

Error Handling and Return Codes

Library functions indicate errors through return values that must be checked. malloc returns NULL on failure. File operations return NULL, -1, or EOF for errors. System calls return -1 and set errno. Code that ignores these return values misses error conditions that cause later failures.

errno provides error details for functions that set it. After a failed system call, checking errno indicates what went wrong. The pattern if (result == -1) { perror("operation"); } logs meaningful error messages. When we see error handling without errno checks, diagnostics suffer.

Error paths must clean up resources. When a function allocates memory, opens files, or acquires resources, error paths must free those resources before returning. The pattern uses goto for cleanup: error: free(ptr); close(fd); return -1;. When error paths miss cleanup, resources leak on failures.

Failed allocations shouldn't leak previously allocated memory. The pattern allocates, checks for NULL, and frees previous allocations on failure. When we see multiple allocations without error handling between them, earlier allocations leak if later ones fail.

File operations can fail for many reasons. fopen returns NULL, fprintf returns negative on error, fclose returns EOF on error. Production code checks these conditions. When file operations don't check errors, data loss or corruption might go unnoticed.

Resource Acquisition and Cleanup

File handles must be closed on all code paths. The pattern opens files, performs operations, and closes in finally-equivalent pattern. C doesn't have finally blocks, so error paths need explicit cleanup. When we see file opens without guaranteed closes, descriptors leak.

Early returns bypass cleanup code that follows them. When a function returns early from an error condition, any cleanup that would happen later is skipped. The goto cleanup pattern handles this: early returns goto a cleanup label that closes files and frees memory.

fclose and close can fail and should have return values checked. Closing might fail due to buffering issues or disk full. Code that ignores close errors might miss data loss. When we see closes without error checking, that's a potential data loss.

Temporary files should be deleted on error paths. Creating temporary files for processing requires cleanup even when operations fail. We verify temporary file cleanup happens on all paths including errors.

Mutexes and locks must be released on all code paths. Forgetting to unlock creates deadlocks when other threads try to acquire the lock. The pattern acquires, performs operations, and releases even on errors. When lock acquisition lacks corresponding release on all paths, deadlocks occur.

Undefined Behavior Prevention

Uninitialized variables contain garbage that causes unpredictable behavior. Declarations should initialize variables: int count = 0;. Reading uninitialized variables is undefined behavior even if the value isn't used. When we see variables used before assignment, that's undefined behavior.

Signed integer overflow is undefined behavior in C. The expression INT_MAX + 1 doesn't wrap to INT_MIN—it's undefined. When integer calculations might overflow, using unsigned types or checking bounds prevents undefined behavior. Code performing arithmetic without overflow checks can optimize incorrectly.

Dereferencing null or invalid pointers causes undefined behavior. Even if code doesn't use the value, the act of dereferencing is undefined. Pattern if (ptr) *ptr = value; checks before dereference. Dereferencing before checking invokes undefined behavior.

Shift operations by amounts >= type width cause undefined behavior. The expression 1 << 32 on 32-bit int is undefined. Valid shifts are 0 to width-1. When we see shift amounts that might equal or exceed type width, that's undefined behavior.

Reading uninitialized memory from allocations is undefined. malloc doesn't initialize memory, so reading before writing is undefined behavior. When we see allocated memory read before any write, that's undefined behavior.

Strict aliasing violations cause undefined behavior. Accessing object through pointer of incompatible type except char pointer violates strict aliasing. The compiler assumes pointers to different types don't alias, enabling optimizations that break aliasing code.

Bringing It Together

Effective C code review requires vigilance about memory safety and undefined behavior. C provides no safety nets—every pointer dereference, memory operation, and resource access can fail or cause undefined behavior if not handled correctly.

Not all issues are equally severe. A missing const annotation is stylistic. A buffer overflow is a security vulnerability. Missing free in error path is a production issue. The key is prioritizing issues that affect safety, security, and reliability over those that represent better practices.

C remains essential for systems programming despite its hazards. Modern tools like static analyzers, sanitizers, and fuzzers catch many issues code review might miss. When reviewing C, we verify code uses these tools and addresses their findings. Combined with careful review of memory management, pointer usage, and error handling, we can write reliable C code that handles production loads safely.