Working in an Existing Codebase

1The first hour

Resist the urge to open the file named in the bug report. Spend the first hour on orientation; it pays for itself immediately.

1. Build it. Before anything else. A codebase you cannot compile is one you cannot test a hypothesis against.
2. Run the tests. Note which already fail — otherwise you will spend an afternoon on a failure you did not cause.
3. Read the directory structure. Five minutes with ls and cloc reveals the architecture.
4. Read the public headers. They are the intended summary of what the code does.
5. Find the entry point and follow one path end to end.

cloc .                          # size, languages, where the weight is
ls -R src include | head -50
git log --oneline -20           # what is being worked on now
git shortlog -sn | head         # who to ask
wc -l src/*.c | sort -n | tail  # the biggest files are usually the core

2Navigation

# grep, always available
grep -rn "symbol" --include='*.c' --include='*.h' .
grep -rn "\bparse_config\b" .              # word boundary: fewer false hits
grep -rln "TODO\|FIXME\|XXX" src/          # known soft spots

# ctags: jump to definitions
ctags -R .
vim -t parse_config                        # or Ctrl-] on the identifier

# a language server: exact, not textual
bear -- make                               # produce compile_commands.json
# then clangd in any editor: go to definition, find all references

The difference matters. grep finds the text count in comments, in other identifiers, and in strings. A language server knows which count you mean, follows it through macros, and finds every reference and no others. On a large codebase that is the difference between a list of four hundred hits and a list of six.

The fourth row deserves emphasis. Static analysis of C is limited by function pointers, macros, and conditional compilation. A breakpoint and a backtrace answer "who calls this" definitively, in one run.

3Reading before writing

Before changing a function, establish four things. None is usually written down.

Three call sites is a rule of thumb worth following literally. One tells you what is possible; three tell you what is normal.

4git bisect

"It worked in version 2.1 and fails in 2.4" — with four hundred commits in between. Bisection finds the guilty one in about nine steps by binary search over history.

git bisect start
git bisect bad                  # the current commit is broken
git bisect good v2.1            # this one worked
# git checks out a commit in the middle
make && ./run_the_test
git bisect good                 # or: git bisect bad
# repeat until git names the commit
git bisect reset                # return to where you started

Automate it and the whole search takes one command:

git bisect start HEAD v2.1
git bisect run ./check.sh       # exit 0 = good, non-zero = bad

#!/bin/sh
# check.sh — exit 0 if the build is good
make clean && make >/dev/null 2>&1 || exit 125   # 125 = skip, untestable
./prog test_input | grep -q "expected output"

Exit code 125 marks a commit as untestable — it does not build, so it is skipped rather than blamed. Without it, a broken intermediate commit derails the search.

Bisection is the single highest-leverage technique in this session. Nine builds against reading four hundred diffs, and the result is a specific commit with a message and an author.

5The minimal change

Once you know the cause, the temptation is to fix everything nearby. Resist it, for reasons that are practical rather than aesthetic.

The rules that follow:

• One change per commit. A fix and a cleanup are two commits even if they touch one line each.
• No drive-by reformatting. A whitespace change across a file turns a one-line diff into an unreviewable one. If the file needs reformatting, that is a separate commit, ideally its own pull request.
• Match the local style even where you dislike it. Consistency within a file beats correctness of style.
• Add a regression test. A fix without one will be undone within a year.
• Explain the why in the commit message. The diff shows what changed; only you know why.

Fix use-after-free when config reload fails

parse_config() freed the old config before validating the new one,
so a malformed file left the global pointer dangling and the next
request dereferenced freed memory.

Validate first, then swap and free. Adds a regression test that
reloads with a syntax error and checks the old config survives.

Fixes: #1234

6Worked example: a regression, found and fixed

Build a small repository with a bug buried in its history, then find it the way you would at work.

mkdir -p bisect-demo && cd bisect-demo && git init -q

Commit 1 — a working version:

cat > stats.c <<'EOF'
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* Returns the mean of n values. Caller guarantees n > 0. */
static double mean(const int *v, size_t n)
{
    long total = 0;
    for (size_t i = 0; i < n; i++) total += v[i];
    return (double)total / (double)n;
}

static int maximum(const int *v, size_t n)
{
    int best = v[0];
    for (size_t i = 1; i < n; i++) if (v[i] > best) best = v[i];
    return best;
}

int main(int argc, char *argv[])
{
    if (argc < 2) { fprintf(stderr, "usage: %s N...\n", argv[0]); return 2; }

    size_t n = (size_t)(argc - 1);
    int *v = malloc(n * sizeof *v);
    if (v == NULL) return 1;
    for (size_t i = 0; i < n; i++) v[i] = atoi(argv[i + 1]);

    printf("count %zu mean %.2f max %d\n", n, mean(v, n), maximum(v, n));
    free(v);
    return 0;
}
EOF
cat > check.sh <<'EOF'
#!/bin/sh
gcc -std=c17 -Wall -Wextra -o stats stats.c 2>/dev/null || exit 125
./stats 1 2 3 | grep -q "count 3 mean 2.00 max 3"
EOF
chmod +x check.sh
git add . && git commit -qm "Add stats tool"

Now ten commits of ordinary churn, with the bug introduced in the middle:

for i in 1 2 3; do
  echo "/* comment $i */" >> stats.c
  git commit -qam "Document internals, part $i"
done

# the regression: a bounds change that looks like a cleanup
sed -i 's/for (size_t i = 1; i < n; i++) if (v\[i\]/for (size_t i = 1; i <= n; i++) if (v[i]/' stats.c
git commit -qam "Simplify the maximum loop"

for i in 4 5 6; do
  echo "/* comment $i */" >> stats.c
  git commit -qam "Document internals, part $i"
done

Confirm it is broken, then bisect:

./check.sh; echo "check says $?"      # non-zero under a sanitizer build

git bisect start HEAD HEAD~7
git bisect run ./check.sh

a1b2c3d is the first bad commit
    Simplify the maximum loop

 stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Seven commits, three builds, and the answer is a specific commit with its diff. Reading seven diffs by hand would have worked here; reading four hundred would not, and the procedure is identical.

git bisect reset

Make the sanitizer do the detecting

The bug reads one element past the end. Without instrumentation the output is often still correct, because the byte past the array happens to be small — the silent case from week 12. Strengthen the check:

cat > check.sh <<'EOF'
#!/bin/sh
gcc -std=c17 -Wall -Wextra -g -fsanitize=address -o stats stats.c 2>/dev/null \
    || exit 125
./stats 1 2 3 2>&1 | grep -q "count 3 mean 2.00 max 3" || exit 1
./stats 1 2 3 2>&1 | grep -q "AddressSanitizer" && exit 1
exit 0
EOF

Now the check fails on the memory error rather than on the visible output, so it catches the commit even when the wrong answer happens to look right. A bisect script is only as good as its test — which is the same point week 35 made about CI.

Investigate before fixing

git show a1b2c3d                       # what exactly changed
git log -S "i <= n" --oneline          # was this pattern used elsewhere?
grep -rn "maximum(" .                  # who calls it
git blame -L 12,20 stats.c             # who wrote the surrounding lines

Answer the four questions from section 3 before touching anything. maximum's precondition is n > 0 — visible from v[0] in the first line, and from the single caller checking argc < 2. Its postcondition is an element of the array. That is enough to know the fix is the bound and nothing else.

The minimal change

sed -i 's/i <= n; i++) if (v\[i\]/i < n; i++) if (v[i]/' stats.c

One character. Now resist the four other things you noticed: atoi should be strtol (week 23), mean's precondition is undocumented, the error message could name the program, and the comments are noise. Every one is a real improvement and none belongs in this commit.

git add stats.c
git commit -q -F - <<'EOF'
Fix off-by-one in maximum()

Commit a1b2c3d changed the loop bound from i < n to i <= n while
simplifying, so maximum() reads one element past the end of the
array. AddressSanitizer reports a heap-buffer-overflow; without it
the wrong value is usually plausible, which is why it was not
noticed in review.

Restore the half-open bound. Adds the sanitizer build to check.sh
so this class of defect fails the check rather than passing it.
EOF

The message says what broke, when, why it was missed, and what prevents a recurrence. A reviewer can approve it without reconstructing any of that.

Then open a separate issue for the rest

git checkout -b cleanup-stats
# atoi → strtol with error checking, document the preconditions
git commit -qam "Validate numeric arguments with strtol"

A separate branch and a separate review. The fix can be merged and backported today; the cleanup can be discussed at leisure. Coupling them delays the fix for the sake of the cleanup, which is the wrong trade every time.

7Common mistakes

9Where this leads

Week 44 submits this change: branching, the review workflow, and reviewing someone else's C against a memory-safety checklist. The regression test added here is what makes the change reviewable, and the small scope is what makes the review possible at all.