Modernizing Legacy C++ Incrementally: Safety Nets, Ownership Migration and Macro Removal

By 0xmote · Jun 17, 2026 · 22 min read · Updated Sep 29, 2026 Advanced Concept

Why incremental beats a rewrite

Legacy C++ usually mixes raw owning pointers, malloc/free, preprocessor constants, C-style casts and a build system nobody fully understands. The tempting response is to start over. The problem with a rewrite is not effort, it is that the old code encodes years of decisions — odd edge cases, workarounds for specific inputs, compatibility with files written by old versions — and almost none of that is written down. A rewrite discards all of it at once and then rediscovers it one bug report at a time.

Incremental modernization flips the order: first build something that tells you when behavior changes, then make many small structural changes, each of which can be reviewed, tested and reverted on its own. The code is shippable after every step. This post walks through that sequence: what to fix first, how to set up the safety net, and the specific migrations (ownership, macros, enums, builds) with the pitfalls each one hides.

What legacy code usually looks like

Issue	Typical consequence	Modern replacement
Owning raw pointers	Leaks, double frees, use-after-free	`std::unique_ptr`, containers, RAII wrappers
`malloc`/`free`, manual buffers	Leaks on early return or exception	`std::vector`, `std::string`
`#define` constants and function macros	No types, no scope, name collisions	`constexpr`, `inline` functions, templates
Unscoped `enum`	Names leak into the enclosing scope, implicit int conversion	`enum class`
Implicit conversions	Silent narrowing, wrong overload chosen	`explicit` constructors, strong types
Unsynchronized globals	Data races	`std::atomic`, `std::mutex`, ThreadSanitizer
Hand-written Makefiles	Missing header dependencies, stale builds	Target-based CMake
No tests	Every change is a gamble	Characterization and golden-output tests

A small example that has several of these at once:

// Legacy style
#define MAX_SIZE 1024
#define LOG(msg) printf("%s\n", msg)

class OldClass {
    char* buffer;
    FILE* file;
public:
    OldClass() {
        buffer = (char*)malloc(MAX_SIZE);
        file = fopen("data.txt", "r");   // may return NULL
    }
    ~OldClass() {
        free(buffer);
        fclose(file);                    // undefined behavior if file is NULL
    }
    void process() {
        char* temp = (char*)malloc(100);
        // ... early return somewhere below leaks temp
    }
    // No copy constructor: copying OldClass copies the pointers,
    // and both copies free the same buffer in their destructors.
};

The last comment is the one people miss. A class that owns resources but does not define or delete its copy operations gets the compiler-generated shallow copy, so passing it by value anywhere produces a double free. That is the “rule of three” problem, and it is one of the first things worth hunting for.

Deciding what to fix first

Not all debt is equal, and “code that looks old” is a poor priority signal. A useful order:

Security and memory-safety bugs — unbounded strcpy/sprintf into fixed buffers, unchecked indices.
Known crash sites — anything in crash reports or sanitizer output.
Build breakage and flakiness — if the build is unreliable, nothing else can be verified.
High-churn code without tests — git log --format= --name-only | sort | uniq -c | sort -rn | head lists the files that change most often; those are where regressions will land next.
Style debt in stable code — a 15-year-old module that never changes and never crashes can keep its macros.

// Priority 1: security
void handle_input(const char* input) {
    char buffer[100];
    strcpy(buffer, input);   // overflow for inputs >= 100 bytes
}

// Priority 2: crash site
void process_data(Data* data) {
    data->value++;           // callers sometimes pass nullptr
}

// Priority 5: low-risk style debt
#define OLD_CONSTANT 42       // replace when the file is next touched

To find candidates, combine several signals: compiler warnings (-Wall -Wextra), a static analyzer, sanitizer runs of the existing test suite (however small), and coverage from gcov/lcov to see which critical paths nothing exercises.

Phase 1: build the safety net

Characterization tests

A characterization test does not assert what the code should do; it records what it currently does. You call the function with a spread of inputs, including odd ones, and write down the results as expectations — even the ones that look wrong.

TEST(LegacyParser, CharacterizesCurrentBehavior) {
    EXPECT_EQ(parse_amount("12.50"), 1250);
    EXPECT_EQ(parse_amount(""), 0);        // arguably a bug, but callers may rely on it
    EXPECT_EQ(parse_amount("1,000"), 1);   // stops at the comma today; pinned deliberately
}

Pinning “wrong” behavior feels odd, but it is the point. During a refactor you want any change to fail a test, so that behavior changes are always intentional and reviewed separately.

For code that writes files, reports or logs, a golden-output test is often cheaper than individual assertions: run the legacy path once, save the output as a reference file, and compare byte for byte afterwards.

TEST(LegacyReport, MatchesGoldenOutput) {
    std::ostringstream out;
    legacy_generate_report(sample_input(), out);
    EXPECT_EQ(out.str(), read_file("golden/report.txt"));
}

Golden files need care with anything non-deterministic — timestamps, hash-map iteration order, floating-point formatting across compilers. Normalize those before comparing, or the test becomes noise that people learn to regenerate without looking.

Sanitizers and static analysis in CI

# AddressSanitizer + UndefinedBehaviorSanitizer for a test build
cmake -B build-asan -DCMAKE_BUILD_TYPE=Debug \
      -DCMAKE_CXX_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -g"
cmake --build build-asan && ctest --test-dir build-asan

# clang-tidy with a focused check set instead of '*'
clang-tidy -p build src/foo.cpp \
  --checks='-*,bugprone-*,modernize-use-nullptr,modernize-use-override,modernize-make-unique'

Enabling every clang-tidy check (--checks='*') on a legacy codebase produces thousands of findings, many from mutually contradictory style checks, and the result is that nobody reads them. Starting with bugprone-* plus a few mechanical modernize-* checks gives a list that is short enough to act on. Several modernize checks can apply fixes automatically with -fix, which is the cheapest modernization there is — but review the diff, and land each check’s fixes as its own commit.

Warnings follow the same logic. Turning on -Werror for a codebase that currently emits 800 warnings just breaks the build. A more workable approach is to fix warnings per directory, and enable -Werror only for directories that are already clean, so the clean area can only grow.

Phase 2: put a modern boundary around old code

Before changing internals, it often helps to give callers a clean interface, so that new code stops depending on the legacy API directly:

class TextProcessor {
public:
    virtual ~TextProcessor() = default;
    virtual std::string process(std::string_view input) = 0;
};

class LegacyTextProcessor : public TextProcessor {
    LegacyClass& legacy_;   // not owned
public:
    explicit LegacyTextProcessor(LegacyClass& l) : legacy_(l) {}
    std::string process(std::string_view input) override {
        std::string arg(input);                        // legacy needs a NUL-terminated string
        char* raw = legacy_.old_process(arg.c_str());  // returns malloc'd memory
        std::unique_ptr<char, decltype(&std::free)> guard(raw, &std::free);
        return raw ? std::string(raw) : std::string();
    }
};

Two details are worth copying. std::string_view is not guaranteed to be NUL-terminated, so it has to be copied into a std::string before being handed to a C-style API that expects const char*. And the returned buffer is released through a unique_ptr with std::free as the deleter, so an exception in the std::string constructor cannot leak it. The memory came from malloc, so it must go back through free, not delete.

Phase 3: migrate ownership

From manual buffers to containers

// Legacy
void process() {
    char* buffer = (char*)malloc(1024);
    if (error_condition) return;   // leak
    // ... use buffer ...
    free(buffer);
}

// Modern
void process() {
    std::vector<char> buffer(1024);
    if (error_condition) return;   // freed automatically
    // ... use buffer.data() where a char* is needed ...
}

std::vector value-initializes its elements (to zero), while malloc does not. That is almost always harmless, but in a hot loop allocating large buffers it is measurable, and in code that accidentally read uninitialized memory it can change output — which is exactly what your characterization tests are there to notice.

From owning raw pointers to unique_ptr

// Legacy: who owns resource? Copying Manager double-deletes it.
class Manager {
    Resource* resource;
public:
    Manager() : resource(new Resource()) {}
    ~Manager() { delete resource; }
};

// Modern: ownership is in the type, copying is a compile error, moving works.
class Manager {
    std::unique_ptr<Resource> resource;
public:
    Manager() : resource(std::make_unique<Resource>()) {}
};

The migration also changes semantics: the modern Manager is move-only. Any code that copied it — which was a latent double free — now fails to compile. That is good, but it means one small change can produce a long list of compile errors in callers, and each needs a decision: should it move, share, or hold a reference instead?

A hazard I have seen more than once is migrating a pointer to unique_ptr while some legacy function elsewhere still frees it. A C-style API that takes a pointer and “consumes” it (calls free or delete internally) will now free memory that the unique_ptr also frees at scope exit. It usually does not crash at the point of the bug; it corrupts the heap and crashes later somewhere unrelated. The fix is to find every function that takes ownership, name it in the signature (std::unique_ptr<T> by value, or release() at the call with a comment), and run the tests under AddressSanitizer, which reports double frees and malloc/delete mismatches (alloc-dealloc-mismatch) at the exact call site instead of wherever the heap finally falls over.

A related trap: replacing malloc(size) with std::unique_ptr<char[]>(new char[size]) is only safe if nothing else calls free or realloc on that pointer. Memory from new[] must be released with delete[], and mixing allocation families is undefined behavior even when it appears to work on one platform.

Step by step, not all at once

class MigratedClass {
    std::unique_ptr<Data> data_;  // migrated
    OldClass* old_;               // not yet: owner still unclear
public:
    void setData(std::unique_ptr<Data> d) { data_ = std::move(d); }
};

// callers make the transfer visible
auto data = std::make_unique<Data>();
obj.setData(std::move(data));

Leaving old_ as a raw pointer for now is fine. A raw pointer is a perfectly good non-owning pointer; the goal is not zero raw pointers, it is that every owning pointer is a smart pointer, so that a remaining raw pointer can be read as “I do not own this”.

When changing function parameters, match the old contract. Replacing void process(char* data) with void process(std::string& data) changes behavior if the function mutated a caller’s fixed buffer in place, and changes the ABI for anything linking against it. For read-only text, std::string_view (or const std::string&) is the faithful replacement; for a mutable buffer, std::span<char> in C++20 keeps the in-place semantics.

Phase 4: macros, constants and enums

Macros to constexpr

// Legacy
#define MAX_SIZE 1024
#define MIN(a, b) ((a) < (b) ? (a) : (b))

// Modern
constexpr int kMaxSize = 1024;
// MIN(a, b) -> std::min(a, b) from <algorithm>

The trap is doing this while any header still defines the old macro. If #define MAX_SIZE 1024 survives in some header and you write constexpr int MAX_SIZE = 1024; in a file that includes it, the preprocessor rewrites your line to constexpr int 1024 = 1024; and GCC reports:

error: expected unqualified-id before numeric constant
    1 | #define MAX_SIZE 1024
      |                  ^~~~
note: in expansion of macro 'MAX_SIZE'

That is why the example uses a new name (kMaxSize) rather than reusing the macro’s. Also search for #if and #ifdef uses of the macro before deleting it: #if MAX_SIZE > 512 cannot see a constexpr variable (in #if, an unknown name silently evaluates to 0), so that condition flips without any error. Compiling with -Wundef catches it.

MIN/MAX macros are worth replacing for a different reason: they evaluate the winning argument twice, so MIN(next(), limit) calls next() twice when it is smaller. Replacing them with std::min fixes that silently — a behavior change your tests should see if anything depended on it. Note that std::min(a, b) requires both arguments to have the same type, so std::min(x, 10u) with an int x will fail to compile where the macro quietly compared signed with unsigned.

Unscoped enums to enum class

// Legacy
enum Color { RED, GREEN, BLUE };
Color c = RED;   // RED is visible in the enclosing scope
int x = RED;     // implicit conversion to int

// Modern
enum class Color { Red, Green, Blue };
Color c = Color::Red;
// int x = Color::Red;                   // error: cannot convert
int x = static_cast<int>(Color::Red);    // explicit where the number matters

The number usually matters somewhere: files, network messages or databases that store the enum as an integer. Keep the numeric values identical (spell them out explicitly with = 0, = 1 if the file format depends on them), and never reorder enumerators in a type that is serialized.

Phase 5: the build

# Typical legacy Makefile: header dependencies are missing,
# so editing a .h does not rebuild the .o files that include it.
CXX = g++
CXXFLAGS = -Wall -O2
OBJS = main.o utils.o
app: $(OBJS)
	$(CXX) $(CXXFLAGS) -o app $(OBJS)

# Target-based CMake
cmake_minimum_required(VERSION 3.14)
project(MyApp VERSION 1.0 LANGUAGES CXX)

add_executable(app main.cpp utils.cpp)
target_compile_features(app PRIVATE cxx_std_17)
target_compile_options(app PRIVATE -Wall -Wextra)

option(MYAPP_ASAN "Build with AddressSanitizer" OFF)
if(MYAPP_ASAN)
    target_compile_options(app PRIVATE -fsanitize=address -fno-omit-frame-pointer)
    target_link_options(app PRIVATE -fsanitize=address)
endif()

find_package(Boost REQUIRED COMPONENTS filesystem)
target_link_libraries(app PRIVATE Boost::filesystem)

The missing-header-dependency problem in the Makefile is not cosmetic: it produces builds where some object files were compiled against an old version of a struct, which shows up as memory corruption that disappears after make clean. CMake-generated builds track header dependencies automatically. Sanitizer flags go in both compile and link options (target_link_options needs CMake 3.13+); passing them through target_link_libraries works but mixes flags with libraries. Keep -Werror out of options that consumers of your project inherit, since a newer compiler with new warnings would break their builds.

Other migrations that pay off early

// File handling: fclose is skipped if process() throws
void read_file(const char* path) {
    FILE* file = fopen(path, "r");
    if (!file) return;
    char buffer[1024];
    while (fgets(buffer, sizeof(buffer), file)) process(buffer);
    fclose(file);
}

// Modern: ifstream closes itself on every exit path
void read_file(const std::filesystem::path& path) {
    std::ifstream file(path);
    if (!file) throw std::runtime_error("cannot open " + path.string());
    std::string line;
    while (std::getline(file, line)) process(line);
}

Note that the modern version changes one behavior: it throws where the old one silently returned. If callers relied on “missing file means do nothing”, that is a behavior change and belongs in its own PR. Likewise fgets keeps the trailing newline and std::getline strips it, which can change parsing downstream.

// Shared counter: data race
static int counter = 0;
void increment() { counter++; }

// Option A: atomic, enough for an independent counter
std::atomic<int> counter{0};
void increment() { counter.fetch_add(1, std::memory_order_relaxed); }

// Option B: mutex, when several variables must change together
std::mutex m;
int counter = 0;
void increment() { std::lock_guard<std::mutex> lock(m); ++counter; }

memory_order_relaxed is correct for a pure statistics counter, but not if other threads use the counter’s value to decide whether some other data is ready; then you need acquire/release ordering or a mutex.

Mistakes that derail modernization

The big-bang rewrite in disguise. A branch titled “modernize networking” that touches 200 files for six weeks is a rewrite. It cannot be reviewed, it conflicts with everyone else’s work, and when it regresses, nobody can tell which of the hundreds of changes caused it. Aim for pull requests that each do one mechanical thing.

Fixing bugs during a refactor. If a refactor and a bug fix land together and output changes, you cannot tell whether the change is the fix or a regression. Refactor first with behavior preserved, then fix the bug separately with a test that reproduces it.

No way back. For risky replacements of whole subsystems, keep the old path available behind a runtime switch until the new one has proven itself:

Result process(const Request& req) {
    if (flags.is_enabled("new_parser") && new_parser.supports(req)) {
        return new_parser.process(req);
    }
    return old_parser.process(req);
}

This is the “strangler fig” approach: the new implementation takes over one kind of request at a time, and the old one keeps handling everything else until it can be deleted.

A stronger variant is a parallel run: call both implementations, compare, log disagreements, and return the old result until the logs are clean.

Result process(const Request& req) {
    Result old_result = old_impl(req);
    Result new_result = new_impl(req);
    if (old_result != new_result) log_discrepancy(req, old_result, new_result);
    return old_result;
}

This only works if the new implementation has no side effects — if both write to the database or send the message, you have doubled the side effects. It also doubles the cost of the call, so it is usually sampled on a fraction of traffic rather than enabled everywhere.

When I have seen modernization efforts stall, the cause was rarely technical. It was usually a long-running branch that fell too far behind, or a flood of analyzer warnings that everyone learned to ignore. Keeping every step small enough to merge the same week is what keeps the effort alive.

Next: C++ career roadmap (#45-3) Previous: Rust interop (#44-2)

Frequently Asked Questions (FAQ)

Q. I found a bug while refactoring legacy code. Should I fix it in the same PR?

A. No. Keep the refactor behavior-preserving and fix the bug in a separate PR with a test that reproduces it. If both land together and something regresses, you cannot tell whether the new structure or the behavior change caused it, and reverting one means reverting both. Some callers may even depend on the buggy behavior, which is another reason to make the fix a visible, reviewable change of its own.