C++ 코드 리뷰 체크리스트 20가지: 메모리 안전·성능·가독성·스레드 안전
이 글의 핵심
리뷰어가 매번 모든 항목을 눈으로 확인할 수는 없으므로, 정적 분석·새니타이저 같은 자동 체크에 맡길 부분과 사람이 직접 봐야 할 부분을 나누는 리뷰 프로세스를 제안합니다. 피드백을 어떻게 작성할지, 팀 체크리스트는 어떻게 만들지도 FAQ로 다룹니다.
메모리 안전 (5개)
C++는 가비지 컬렉터가 없는 언어이므로, 메모리 관리와 관련된 실수는 코드 리뷰에서 가장 먼저, 가장 꼼꼼하게 확인해야 할 항목입니다. 아래 다섯 가지는 리뷰 과정에서 실제로 자주 발견되는 메모리 관련 버그 패턴으로, 대부분 RAII와 스마트 포인터로 근본적으로 예방할 수 있습니다.
메모리 누수 체크
new로 할당한 메모리를 짝이 되는 delete로 해제하지 않으면 그 메모리는 프로그램이 종료될 때까지 회수되지 않는 누수가 됩니다. 리뷰어는 코드에서 new가 등장할 때마다 반드시 대응하는 delete가 모든 실행 경로(정상 종료, 조기 반환, 예외 발생 포함)에서 호출되는지 확인해야 하며, 이 확인 자체가 번거롭고 실수하기 쉽다는 점에서 애초에 vector나 스마트 포인터 같은 RAII 컨테이너로 대체하는 것을 권장하는 것이 훨씬 근본적인 해결책입니다.
// ❌ 누수
void bad() {
int* ptr = new int[100];
// delete[] 없음!
}
// ✅ RAII
void good() {
vector<int> v(100); // 자동 해제
}댕글링 포인터
함수가 지역 변수의 주소를 반환하는 코드는 리뷰에서 가장 눈에 띄게 찾아야 할 위험 신호 중 하나입니다. 함수가 종료되면 지역 변수는 스택과 함께 소멸되므로 반환된 포인터는 더 이상 유효하지 않은 메모리를 가리키게 되고, 이를 역참조하면 미정의 동작으로 이어집니다. 리뷰 시에는 포인터나 참조를 반환하는 함수를 볼 때마다 “이 값이 가리키는 대상이 함수 종료 후에도 살아있는가”를 항상 확인하는 습관이 필요합니다.
// ❌ 위험
int* bad() {
int x = 10;
return &x; // 지역 변수 주소 반환!
}
// ✅ 안전
int good() {
int x = 10;
return x; // 값 반환
}double delete
같은 포인터에 delete를 두 번 호출하는 것은 힙 메모리 관리자의 내부 상태를 손상시켜 크래시나 예측 불가능한 동작으로 이어지는 심각한 버그입니다. 이런 실수는 코드가 복잡해지고 소유권 관계가 불명확해질수록, 특히 같은 포인터가 여러 경로로 전달되는 코드에서 자주 발생합니다. 해제 직후 포인터를 nullptr로 초기화해 두면 실수로 다시 delete를 호출하더라도 nullptr에 대한 delete는 안전하게 아무 일도 하지 않으므로, 이 습관 하나만으로 상당수의 double delete를 예방할 수 있습니다.
// ❌ 크래시
int* ptr = new int(10);
delete ptr;
delete ptr; // double delete!
// ✅ nullptr 설정
int* ptr = new int(10);
delete ptr;
ptr = nullptr;
delete ptr; // 안전다만 nullptr 대입은 그 변수 하나만 보호한다는 한계를 리뷰어가 알고 있어야 합니다. 같은 주소를 다른 포인터 변수나 컨테이너, 다른 객체의 멤버가 복사해 들고 있다면 그쪽은 여전히 해제된 메모리를 가리키고, 나중에 그 사본을 delete하면 똑같이 double delete가 됩니다. 실무에서 double delete는 대부분 이런 “소유권이 불분명한 복사본”에서 나오므로, 리뷰에서 진짜로 물어야 할 질문은 “누가 이 메모리의 주인인가”입니다. 주인이 하나라면 unique_ptr, 정말로 여럿이 공유한다면 shared_ptr로 타입에 소유권을 드러내면 이 질문 자체가 코드에서 답해집니다. 크래시 메시지는 glibc에서 free(): double free detected in tcache 2처럼 나오지만, 해제된 메모리가 이미 다른 용도로 재사용된 뒤라면 전혀 엉뚱한 곳에서 힙 손상으로 터지기도 해서, AddressSanitizer(-fsanitize=address)로 재현하는 것이 가장 빠른 진단 방법입니다.
배열 범위 초과
배열이나 operator[]로 접근하는 컨테이너는 범위를 벗어난 인덱스로 접근해도 즉시 에러를 내지 않고 조용히 잘못된 메모리를 읽거나 써버리므로, 코드 리뷰에서 인덱스 계산 로직을 특히 주의 깊게 살펴봐야 합니다. 리뷰어는 반복문의 종료 조건이 <인지 <=인지, 인덱스가 사용자 입력이나 외부 데이터에서 온 값이라면 범위 검증이 이루어지는지를 확인해야 하며, 범위를 벗어나면 예외를 던지는 at()을 사용하도록 제안하는 것도 좋은 대안입니다.
// ❌ 위험
int arr[10];
arr[10] = 5; // 범위 초과!
// ✅ 체크
if (index < 10) {
arr[index] = 5;
}
// ✅ vector 사용
vector<int> v(10);
v.at(10) = 5; // 예외 발생리뷰에서 자주 보는 변형은 부호 없는 인덱스와 섞인 경계 조건입니다. for (size_t i = v.size() - 1; i >= 0; --i)는 size_t가 음수가 될 수 없어서 i >= 0이 항상 참이 되고, 0에서 1을 빼는 순간 아주 큰 값으로 넘어가 범위 밖을 읽습니다. 빈 벡터에서는 첫 반복부터 그렇습니다. 역순 순회는 for (size_t i = v.size(); i-- > 0;)나 역방향 이터레이터(rbegin())를 쓰도록 제안하는 것이 좋습니다. at()은 매 접근마다 검사 비용이 들기 때문에 성능이 중요한 내부 루프보다는 외부 입력으로 인덱스를 받는 경계 지점에 쓰는 것이 현실적인 절충이고, 개발 중에는 libstdc++의 -D_GLIBCXX_ASSERTIONS나 MSVC 디버그 빌드처럼 operator[]에도 범위 검사를 켜 주는 옵션을 활용할 수 있습니다.
스마트 포인터 사용
새로 작성된 코드에 원시 포인터로 자원을 직접 관리하는 new/delete 쌍이 등장한다면, 리뷰어는 왜 스마트 포인터를 쓰지 않았는지 반드시 물어봐야 합니다. 아래 예시처럼 new와 delete 사이에 복잡한 로직이 끼어들수록 중간에 예외가 발생하거나 조기 반환 경로가 추가될 때 delete를 빠뜨리기 쉬워지지만, std::unique_ptr나 std::shared_ptr로 바꾸면 이런 실수 가능성 자체가 컴파일러 수준에서 사라집니다.
// ❌ raw 포인터
int* ptr = new int(10);
// ... 복잡한 로직 ...
delete ptr; // 깜빡할 수 있음
// ✅ 스마트 포인터
auto ptr = make_unique<int>(10);
// 자동 해제스마트 포인터로 바꾸라는 코멘트를 남길 때는 어떤 스마트 포인터인지도 함께 봐야 합니다. 리뷰에서 흔히 보는 과잉 교정은 모든 원시 포인터를 shared_ptr로 바꾸는 것인데, shared_ptr는 참조 카운트를 원자적으로 증감하는 비용이 있고, 무엇보다 “누가 주인인지”를 흐리게 만들어 순환 참조로 인한 누수를 새로 만들기도 합니다. 기본값은 unique_ptr이고, 소유권을 넘기지 않고 잠깐 빌려 쓰는 함수 매개변수는 여전히 원시 포인터나 참조(T*, T&)가 맞습니다. 모든 원시 포인터가 나쁜 것이 아니라, 소유하는 원시 포인터가 문제라는 구분을 리뷰 코멘트에 담아 주면 작성자가 같은 실수를 반복하지 않습니다.
성능 (5개)
성능 문제는 메모리 안전성 문제와 달리 당장 프로그램을 망가뜨리지는 않지만, 방치하면 사용자 트래픽이 늘어날수록 병목으로 누적되는 특성이 있습니다. 아래 다섯 가지는 리뷰 중 무심코 지나치기 쉽지만, 짚어주면 즉시 개선할 수 있는 대표적인 성능 패턴입니다.
불필요한 복사
함수 매개변수를 값으로 받으면 호출할 때마다 인자 전체가 복사되는데, 이 비용은 인자가 커질수록(큰 벡터, 문자열, 사용자 정의 클래스 등) 무시할 수 없는 수준이 됩니다. 리뷰어는 함수가 인자를 읽기만 하고 수정하지 않는다면 값 대신 const 참조로 받도록 권장해야 하며, 이 작은 변경만으로도 불필요한 메모리 할당과 복사 비용을 완전히 없앨 수 있습니다.
// ❌ 복사
void process(vector<int> data) { // 복사!
// ...
}
// ✅ 참조
void process(const vector<int>& data) {
// ...
}reserve 사용
std::vector는 용량이 부족해질 때마다 더 큰 메모리를 새로 할당하고 기존 원소를 옮긴 뒤 이전 메모리를 해제하는 재할당을 반복하는데, 최종 크기를 미리 알 수 있는 상황에서는 이런 재할당 자체가 낭비입니다. 반복문 전에 예상되는 최종 크기로 reserve()를 호출해 두면 벡터가 처음부터 충분한 공간을 확보하므로, 반복적인 push_back 과정에서 재할당이 전혀 일어나지 않아 성능이 크게 개선됩니다.
// ❌ 재할당 여러 번
vector<int> v;
for (int i = 0; i < 1000; i++) {
v.push_back(i);
}
// ✅ 미리 할당
vector<int> v;
v.reserve(1000);
for (int i = 0; i < 1000; i++) {
v.push_back(i);
}솔직히 말하면 원소 1000개짜리 예제에서는 차이가 측정하기 어려울 만큼 작습니다. vector는 용량을 배수로 늘리기 때문에 push_back은 재할당을 포함해도 평균 O(1)이고, 1000개면 재할당은 열 번 남짓입니다. reserve가 의미 있는 것은 원소가 크거나(이동 비용이 비싼 객체) 개수가 수십만 이상이거나, 재할당 때문에 기존 원소를 가리키던 포인터·이터레이터가 무효화되는 문제를 피해야 할 때입니다. 반대로 리뷰에서 꼭 잡아야 할 안티패턴이 있는데, 반복문 안에서 v.reserve(v.size() + 1)처럼 조금씩 늘리는 코드입니다. 이렇게 하면 배수 증가 전략이 깨져 매번 재할당이 일어나고 전체가 O(n²)이 됩니다.
적절한 자료구조
std::vector에서 특정 값을 찾으려고 find를 반복 호출하는 코드는 매번 처음부터 끝까지 선형 탐색을 해야 해서 원소 수가 많아질수록 눈에 띄게 느려집니다. 검색이 빈번하게 일어나는 데이터라면 정렬된 트리 구조인 std::set이나 해시 기반의 std::unordered_set으로 바꾸면 각각 O(log n), 평균 O(1)로 검색 성능을 크게 개선할 수 있으며, 리뷰어는 반복적인 find 호출을 볼 때마다 자료구조 선택이 최선인지 되짚어 볼 필요가 있습니다.
// ❌ 느림 (O(n) 검색)
vector<int> v;
find(v.begin(), v.end(), x);
// ✅ 빠름 (O(log n) 또는 O(1))
set<int> s;
s.find(x);
unordered_set<int> us;
us.find(x);이 항목은 “무조건 set으로 바꾸라”가 아니라 접근 패턴을 확인하라는 뜻입니다. 원소가 수십 개 정도라면 연속된 메모리를 순서대로 읽는 vector의 선형 탐색이 캐시 효율 덕분에 set의 트리 탐색보다 빠른 경우가 흔합니다. set은 노드마다 별도로 할당되어 메모리를 여기저기 건너뛰며 읽기 때문입니다. 데이터를 한 번 만들고 여러 번 검색한다면 정렬된 vector에 lower_bound를 쓰는 것이 트리보다 빠르고 메모리도 적게 씁니다. unordered_set은 평균 O(1)이지만 해시 계산 비용이 있고, 적대적인 입력으로 해시 충돌을 유도하면 최악의 경우 O(n)이 된다는 점도 외부 입력을 다루는 코드라면 고려해야 합니다.
문자열 연결
문자열 연결에서 리뷰어가 실제로 잡아야 할 것은 += 자체가 아니라 result = result + s처럼 매번 새 임시 문자열을 만드는 패턴입니다. 이 형태는 반복할 때마다 지금까지 쌓인 문자열 전체를 복사한 새 객체를 만들고 버리므로 전체 비용이 O(n²)으로 커집니다. 반면 result += s는 vector처럼 용량을 배수로 늘려 가며 제자리에 이어 붙이기 때문에 평균적으로 이미 효율적이고, 최종 길이를 대략 안다면 reserve()로 재할당까지 없앨 수 있습니다.
// ❌ 느림: 매 반복마다 전체를 복사한 임시 문자열 생성
string result;
for (const auto& s : strings) {
result = result + s;
}
// ✅ 빠름: 필요한 크기를 한 번 확보하고 제자리에서 이어 붙이기
size_t total = 0;
for (const auto& s : strings) total += s.size();
string result;
result.reserve(total);
for (const auto& s : strings) {
result += s;
}std::ostringstream은 숫자나 여러 타입을 섞어 형식화할 때 편리하지만, 단순히 문자열을 이어 붙이는 용도로는 스트림 상태 관리와 로케일 처리 비용 때문에 +=보다 느린 경우가 많습니다(이 항목의 이전 버전은 반대로 설명하고 있었습니다). 형식화가 필요하다면 C++20의 std::format이 ostringstream보다 빠르고 읽기도 쉽습니다. 성능 코멘트를 남길 때는 이런 통념보다 실제 측정 결과를 근거로 드는 편이 설득력이 있습니다.
move 시맨틱스
더 이상 사용하지 않을 객체를 다른 변수에 대입할 때 std::move를 쓰지 않으면 컴파일러는 안전하게 복사를 선택하며, 이는 큰 컨테이너나 문자열에서 불필요한 메모리 할당과 복사 비용으로 이어집니다. 리뷰어는 원본 변수를 대입 이후 다시 사용하지 않는다는 것이 확실한 지점을 발견하면 std::move로 자원 이동을 명시적으로 유도하도록 제안해, 복사 대신 포인터 이전만으로 대입이 끝나도록 개선할 수 있습니다.
// ❌ 복사
vector<int> v1 = getData();
vector<int> v2 = v1; // 복사
// ✅ move
vector<int> v1 = getData();
vector<int> v2 = move(v1); // movemove 이후의 v1은 “유효하지만 지정되지 않은 상태”입니다. 표준 컨테이너의 경우 실제로는 대부분 비어 있지만 표준이 보장하는 것은 아니므로, 이동한 변수를 다시 읽는 코드가 보이면 리뷰에서 지적해야 합니다(clang-tidy의 bugprone-use-after-move가 이를 잡아 줍니다). 새 값을 대입하거나 clear()한 뒤 다시 쓰는 것은 괜찮습니다.
반대 방향의 실수도 흔합니다. 함수가 지역 변수를 반환할 때 return move(result);라고 쓰면 오히려 컴파일러의 반환값 최적화(NRVO)를 막아 불필요한 이동이 생깁니다. return result;만 쓰면 컴파일러가 복사 자체를 생략하거나, 못 하더라도 자동으로 이동을 선택합니다. GCC/Clang은 이 경우 -Wpessimizing-move 경고를 내므로, “move를 더 붙이면 빨라진다”는 제안보다 이 경고를 따르는 편이 정확합니다. const 객체에 move를 쓰는 것도 조용히 복사로 바뀌는 대표적인 함정입니다.
가독성 (5개)
가독성은 코드가 당장 동작하느냐와는 별개로, 몇 달 뒤 다른 사람(또는 미래의 자신)이 그 코드를 이해하고 안전하게 수정할 수 있느냐를 좌우합니다. 아래 다섯 가지는 가독성을 해치는 대표적인 습관들로, 대부분 사소해 보이지만 누적되면 코드베이스 전체의 유지보수 비용을 크게 늘립니다.
명확한 변수명
d나 tmp처럼 의미를 알 수 없는 짧은 변수명은 코드를 작성한 시점에는 문맥이 뻔해 보이지만, 나중에 코드를 다시 읽을 때는 그 의미를 다시 추론해야 하는 인지적 부담을 만듭니다. 리뷰어는 변수명만 보고 그 역할을 짐작할 수 없다면 daysUntilExpiry, userCount처럼 값이 무엇을 나타내는지 드러나는 이름으로 바꾸도록 요청해야 하며, 이는 코드 자체가 최소한의 문서 역할을 하게 만드는 가장 저렴한 투자입니다.
// ❌ 불명확
int d; // 뭘 의미?
int tmp;
// ✅ 명확
int daysUntilExpiry;
int userCount;함수 길이
한 함수가 100줄이 넘어가면 그 함수가 정확히 무엇을 하는지 한눈에 파악하기 어려워지고, 함수 안의 지역 변수와 조건 분기가 서로 얽히면서 수정할 때 부작용을 예측하기도 힘들어집니다. 리뷰어는 함수가 여러 단계의 서로 다른 책임(검증, 변환, 저장 등)을 한꺼번에 처리하고 있다면 각 단계를 이름이 명확한 작은 함수로 분리하도록 제안해야 하며, 이렇게 나누면 각 함수를 개별적으로 테스트하고 재사용하기도 훨씬 쉬워집니다.
// ❌ 너무 긴 함수 (100줄+)
void processData() {
// ... 100줄 ...
}
// ✅ 작은 함수로 분리
void validateData() { /* ... */ }
void transformData() { /* ... */ }
void saveData() { /* ... */ }
void processData() {
validateData();
transformData();
saveData();
}매직 넘버 제거
코드 안에 2나 100 같은 숫자가 그대로 등장하면, 그 숫자가 왜 그 값이어야 하는지 코드만 봐서는 알 수 없어 나중에 값을 바꿔야 할 때 어디를 고쳐야 할지 찾기도 어렵습니다. 리뷰어는 이런 매직 넘버를 발견하면 이름 붙은 상수나 enum class로 바꾸도록 요청해야 하며, 이렇게 하면 값의 의미가 이름으로 드러날 뿐 아니라 같은 값을 여러 곳에서 참조할 때 한 곳만 수정하면 되는 유지보수 이점도 함께 얻을 수 있습니다.
// ❌ 매직 넘버
if (status == 2) { // 2가 뭐지?
// ...
}
// ✅ 상수
const int STATUS_ACTIVE = 2;
if (status == STATUS_ACTIVE) {
// ...
}
// ✅ enum
enum class Status { Inactive, Pending, Active };
if (status == Status::Active) {
// ...
}주석
좋은 주석은 코드가 “무엇을” 하는지가 아니라 “왜” 그렇게 했는지를 설명해야 하는데, 코드를 그대로 한국어로 옮긴 듯한 주석이나 몇 년째 방치된 TODO는 오히려 코드를 읽는 데 방해가 됩니다. 리뷰어는 주석이 코드만 봐도 알 수 있는 내용을 반복하고 있다면 삭제를 제안하고, 반대로 왜 특정 값이나 방식을 선택했는지에 대한 배경 설명이 빠져 있다면 그 의도를 주석으로 남기도록 요청하는 것이 바람직합니다.
// ❌ 불필요한 주석
int x = 10; // x에 10을 할당
// ❌ 오래된 주석
// TODO: 나중에 수정 (2020년)
// ✅ 의도 설명
// 타임아웃을 10초로 설정 (서버 응답 시간 고려)
int timeout = 10;const 정확성
함수가 매개변수를 읽기만 하고 수정하지 않는다면 그 사실을 const로 명시하는 것이 좋은데, 이는 단순한 스타일 문제가 아니라 함수를 호출하는 쪽과 유지보수하는 쪽 모두에게 “이 함수는 인자를 바꾸지 않는다”는 계약을 코드 자체로 알려주는 역할을 합니다. 리뷰어는 참조나 포인터 매개변수를 볼 때마다 함수 내부에서 실제로 값을 변경하는지 확인하고, 변경하지 않는다면 const를 붙이도록 요청해 실수로 값을 변경하는 버그를 컴파일 타임에 방지할 수 있게 해야 합니다.
// ❌ const 없음
void print(vector<int>& v) {
for (int x : v) {
cout << x << " ";
}
}
// ✅ const 추가
void print(const vector<int>& v) {
for (int x : v) {
cout << x << " ";
}
}안전성 (5개)
앞서 다룬 메모리 안전, 성능, 가독성이 코드 품질의 세 축이라면, 마지막으로 안전성은 예상치 못한 입력이나 예외 상황에서도 프로그램이 견고하게 동작하는지를 다룹니다. 아래 다섯 가지는 특히 프로덕션 환경에서 장애로 직결되기 쉬운 항목들입니다.
예외 안전성
new로 자원을 할당한 뒤 그 사이에 예외를 던질 수 있는 연산을 호출하면, 예외가 실제로 발생했을 때 뒤따르는 delete가 실행되지 못하고 건너뛰어져 자원이 누수됩니다. 리뷰어는 원시 자원 관리 코드 사이에 예외를 던질 가능성이 있는 함수 호출이 끼어 있는지 확인해야 하며, 이런 패턴을 발견하면 RAII 컨테이너로 바꿔 예외가 발생하든 정상 종료되든 자원이 항상 안전하게 정리되도록 개선을 제안해야 합니다.
// ❌ 예외 시 누수
void bad() {
int* ptr = new int[100];
riskyOperation(); // 예외 발생 가능
delete[] ptr; // 실행 안 됨!
}
// ✅ RAII
void good() {
vector<int> v(100);
riskyOperation(); // 예외 발생해도 안전
}nullptr 체크
포인터를 매개변수로 받는 함수가 그 포인터를 검증 없이 곧바로 역참조하면, 호출하는 쪽에서 실수로 nullptr을 넘겼을 때 프로그램이 크래시나 미정의 동작으로 이어집니다. 리뷰어는 포인터 매개변수를 받는 함수마다 nullptr 가능성을 검토하고, 검증이 빠져 있다면 명시적인 체크를 추가하거나 애초에 nullptr이 될 수 없는 참조 타입으로 시그니처를 바꾸도록 제안해야 합니다.
// ❌ 체크 없음
void process(int* ptr) {
*ptr = 10; // ptr이 nullptr이면?
}
// ✅ 체크
void process(int* ptr) {
if (!ptr) {
throw invalid_argument("ptr is null");
}
*ptr = 10;
}정수 오버플로우
부호 있는 정수 연산이 표현 가능한 최댓값을 넘어서면 C++ 표준상 미정의 동작이 발생하며, 이는 사용자 입력이나 외부 데이터를 기반으로 계산하는 코드에서 특히 위험합니다. 리뷰어는 덧셈이나 곱셈 같은 연산의 입력값이 외부에서 올 수 있는 경우 오버플로 가능성을 항상 의심해야 하며, 연산 전에 범위를 검사하거나 더 넓은 정수 타입을 사용하도록 제안해 이런 위험을 사전에 차단해야 합니다.
// ❌ 오버플로우 가능
int a = INT_MAX;
int b = a + 1; // 오버플로우!
// ✅ 체크
if (a > INT_MAX - 1) {
throw overflow_error("overflow");
}
int b = a + 1;위 검사는 “1을 더하는” 경우에만 맞는 식이라, 일반적인 a + b라면 if ((b > 0 && a > INT_MAX - b) || (b < 0 && a < INT_MIN - b))처럼 부호에 따라 나눠 검사해야 합니다. 이 식을 매번 손으로 쓰면 그 자체가 실수의 원인이 되므로, GCC/Clang에서는 __builtin_add_overflow(a, b, &result)처럼 오버플로 여부를 반환하는 내장 함수를 쓰는 것이 더 안전합니다. 검사 순서도 중요합니다. if (a + b < a)처럼 먼저 더한 뒤 결과로 오버플로를 판단하는 코드는 오버플로 자체가 정의되지 않은 동작이라, 최적화 컴파일러가 “부호 있는 덧셈은 넘치지 않는다”고 가정하고 검사를 통째로 지워 버릴 수 있습니다. 리뷰에서 이 패턴을 보면 반드시 짚어야 합니다. 테스트 단계에서는 -fsanitize=undefined로 실행 중 발생하는 오버플로를 잡을 수 있습니다.
리뷰에서 특히 눈여겨볼 곳은 크기 계산입니다. count * sizeof(T)로 할당 크기를 계산하는데 count가 외부 입력이면, 곱셈이 넘쳐 작은 값이 되고 작은 버퍼를 할당한 뒤 큰 데이터를 써 넣는 힙 오버플로 취약점이 됩니다. 실제 보안 취약점 보고서에서 반복해서 등장하는 패턴입니다.
입력 검증
setter나 생성자가 전달받은 값을 아무런 검증 없이 그대로 저장하면, 논리적으로 불가능한 값(음수 나이, 범위를 벗어난 값 등)이 객체 상태로 들어와 나중에 이 값을 사용하는 다른 코드에서 예상치 못한 오류를 일으킬 수 있습니다. 리뷰어는 외부에서 들어오는 값을 저장하는 지점마다 유효 범위를 검증하는 코드가 있는지 확인하고, 없다면 유효하지 않은 값에 대해 예외를 던지거나 명확히 거부하도록 요청해 잘못된 상태가 객체 안에 스며들지 않도록 막아야 합니다.
// ❌ 검증 없음
void setAge(int age) {
this->age = age; // 음수도 가능?
}
// ✅ 검증
void setAge(int age) {
if (age < 0 || age > 150) {
throw invalid_argument("invalid age");
}
this->age = age;
}스레드 안전성
여러 스레드가 동기화 장치 없이 같은 전역 변수나 멤버 변수를 동시에 읽고 쓰면 데이터 레이스가 발생하며, 이는 타이밍에 따라 발생 여부가 달라져 일반적인 테스트로는 재현되지 않는 경우가 많습니다. 리뷰어는 코드가 멀티스레드 환경에서 실행될 가능성이 있다면 공유 상태에 대한 접근이 mutex나 atomic으로 보호되고 있는지 반드시 확인해야 하며, 이런 검토는 문제가 실제 운영 환경에서 드물게 재현되는 버그로 나타나기 전에 미리 잡아낼 수 있는 중요한 방어선입니다.
// ❌ 경쟁 조건
int counter = 0;
void increment() {
counter++; // 스레드 안전하지 않음!
}
// ✅ mutex
mutex mtx;
int counter = 0;
void increment() {
lock_guard<mutex> lock(mtx);
counter++;
}counter++는 한 줄이지만 “읽기, 더하기, 쓰기” 세 단계로 실행되어, 두 스레드가 같은 값을 읽고 각자 1을 더해 쓰면 증가가 한 번 사라집니다. 이런 단일 정수 카운터라면 std::atomic<int> counter{0};으로 바꾸는 것이 뮤텍스보다 가볍고 코드도 단순합니다. 반대로 두 개 이상의 변수를 함께 일관되게 바꿔야 한다면(잔액과 거래 내역처럼) 각각을 atomic으로 만들어도 둘 사이의 일관성은 보장되지 않으므로 뮤텍스로 묶어야 합니다. 리뷰에서 뮤텍스를 볼 때 확인할 것은 잠금 범위입니다. 잠금을 쥔 채로 파일 I/O나 네트워크 호출, 다른 모듈의 콜백을 부르면 성능이 떨어지고 교착 상태의 원인이 됩니다. 데이터 레이스는 코드 리뷰만으로 모두 찾기 어려우므로, 멀티스레드 코드의 테스트는 ThreadSanitizer(-fsanitize=thread)로 실행하도록 CI에 넣어 두는 것을 권합니다.
코드 리뷰 프로세스
지금까지 다룬 20가지 체크리스트를 사람이 매번 눈으로 하나씩 확인하는 것은 비효율적이며 실수도 생기기 쉽습니다. 효과적인 코드 리뷰 프로세스는 기계가 잘하는 반복적인 검사는 자동화 도구에 맡기고, 사람은 의도와 설계처럼 맥락이 필요한 부분에 집중하는 방식으로 역할을 나눕니다.
자동 체크
컴파일러 경고, 정적 분석, 코드 포맷팅처럼 규칙이 명확한 검사는 사람이 리뷰하기 전에 자동화 도구로 미리 걸러내는 것이 효율적입니다. -Wall -Wextra -Werror로 컴파일 경고를 에러로 승격시키고, cppcheck나 clang-tidy 같은 정적 분석 도구를 CI에 통합해 두면 앞서 다룬 메모리 누수나 정수 오버플로 같은 문제의 상당수를 사람이 코드를 읽기도 전에 자동으로 잡아낼 수 있습니다.
# 컴파일 경고
g++ -Wall -Wextra -Werror
# 정적 분석
cppcheck --enable=all .
clang-tidy *.cpp
# 포맷 체크 (-i는 파일을 직접 고치므로 CI에서는 검사 모드로)
clang-format --dry-run --Werror *.cpp
# 런타임 검사 (테스트 빌드)
g++ -g -fsanitize=address,undefined ...정적 분석으로 잡을 수 없는 메모리 오류와 정의되지 않은 동작은 새니타이저가 맡습니다. AddressSanitizer는 범위 초과, use-after-free, double delete를, UndefinedBehaviorSanitizer는 정수 오버플로와 잘못된 시프트를, ThreadSanitizer는 데이터 레이스를 실행 중에 잡아 줍니다. 실행 속도가 몇 배 느려지므로 운영 빌드에는 넣지 않고, CI의 테스트 빌드 하나를 새니타이저 빌드로 두는 방식이 일반적입니다. 이 글의 1~4번, 18번, 20번 항목은 대부분 테스트가 해당 경로를 실행하기만 하면 새니타이저가 사람보다 정확하게 찾아냅니다. 도구를 처음 도입할 때는 기존 코드에서 경고가 수백 개 쏟아지는 경우가 많은데, 한꺼번에 고치려 하기보다 “새로 바뀐 코드에서는 경고 0”을 먼저 CI 규칙으로 만들고 기존 경고는 점진적으로 줄여 나가는 편이 현실적입니다.
수동 체크
자동화 도구가 잡아낼 수 없는 부분, 즉 코드가 실제로 요구사항을 만족하는지, 테스트가 실제 시나리오를 충분히 다루는지, 설계가 팀의 아키텍처 방향과 맞는지는 사람이 직접 판단해야 합니다. 아래와 같은 체크리스트를 PR 템플릿에 포함시켜 두면, 리뷰어가 매번 무엇을 확인해야 하는지 놓치지 않고 일관된 기준으로 리뷰할 수 있습니다.
□ 코드가 요구사항을 만족하는가?
□ 테스트가 충분한가?
□ 에러 처리가 적절한가?
□ 성능 문제는 없는가?
□ 보안 취약점은 없는가?
□ 가독성이 좋은가?
□ 문서화가 되어 있는가?피드백 작성
같은 문제를 지적하더라도 피드백을 어떻게 전달하느냐에 따라 리뷰가 팀의 학습 기회가 될 수도, 감정적인 마찰이 될 수도 있습니다. 좋은 피드백은 구체적인 코드 위치와 근거를 함께 제시해 왜 그 변경이 필요한지 상대가 스스로 납득할 수 있게 하는 반면, 문제를 구체적으로 짚지 않고 코드 전체를 뭉뚱그려 평가하는 피드백은 받는 사람이 무엇을 어떻게 고쳐야 할지 알 수 없게 만듭니다.
✅ 좋은 피드백:
"line 42: vector를 const 참조로 받으면 복사를 피할 수 있습니다."
❌ 나쁜 피드백:
"이 코드는 엉망입니다."실전 예시
지금까지 다룬 20가지 체크리스트가 실제 코드 하나에 얼마나 많이 겹쳐서 등장할 수 있는지, 리뷰 전과 리뷰 후 코드를 나란히 비교해 보면 명확하게 드러납니다.
예시 1: 리뷰 전 코드
아래 함수는 짧은 코드 안에 앞서 다룬 체크리스트 항목 중 네 가지 문제를 동시에 안고 있습니다. vector<int> data를 값으로 받아 불필요한 복사가 발생하고, 원시 포인터로 배열을 직접 관리하며, 반복문 종료 조건이 <=라서 배열 범위를 한 칸 초과하고, 마지막으로 배열을 new[]로 할당했으면서 delete[]가 아닌 delete로 해제해 정의되지 않은 동작을 일으킵니다.
void process(vector<int> data) {
int* arr = new int[data.size()];
for (int i = 0; i <= data.size(); i++) {
arr[i] = data[i] * 2;
}
// ... 처리 ...
delete arr; // delete[] 아님!
}문제점:
- 불필요한 복사 (vector)
- raw 포인터 사용
- 범위 초과 (i <= size)
- delete vs delete[]
리뷰어가 이 코드에서 하나 더 짚을 수 있는 것은 int i와 data.size()(부호 없는 size_t)의 비교입니다. -Wall -Wextra에서 comparison of integer expressions of different signedness 경고가 나는데, 이 경고를 무시하는 습관이 있는 팀이라면 바로 옆의 <= 실수도 함께 지나치기 쉽습니다. 또 // ... 처리 ... 부분에서 예외가 나면 마지막의 delete까지 도달하지 못하므로 16번 예외 안전성 문제도 숨어 있습니다. 네 가지로 끝나지 않는다는 점이, 체크리스트를 항목별로 따로 보기보다 한 함수 안에서 겹쳐 보는 연습이 필요한 이유입니다.
예시 2: 리뷰 후 코드
리뷰 피드백을 반영한 아래 코드는 앞서 지적된 네 가지 문제를 모두 해결했습니다. 매개변수를 const 참조로 바꿔 복사를 없앴고, 원시 포인터 대신 vector로 결과를 담아 RAII의 이점을 얻었으며, 범위 기반 for문을 사용해 인덱스 계산 실수 자체를 원천적으로 방지했고, reserve()로 결과 벡터의 재할당 비용까지 함께 줄였습니다. 이렇게 하나의 짧은 함수를 고치는 과정만으로도 메모리 안전성, 성능, 가독성이라는 세 가지 체크리스트 카테고리를 동시에 개선할 수 있다는 것을 보여줍니다.
void process(const vector<int>& data) {
vector<int> result;
result.reserve(data.size());
for (int value : data) {
result.push_back(value * 2);
}
// ... 처리 ...
}개선 사항:
- const 참조로 복사 제거
- vector 사용 (RAII)
- 범위 기반 for
- reserve로 성능 개선
FAQ
Q1: 코드 리뷰는 얼마나 자주?
A: 모든 PR/커밋마다 리뷰하는 것이 이상적입니다.
Q2: 리뷰어는 몇 명?
A: 최소 1명, 중요한 코드는 2명 이상 권장합니다.
Q3: 리뷰 시간은?
A: 널리 인용되는 SmartBear/Cisco 사례 연구에서는 한 번에 200400줄 정도를 6090분 이내로 볼 때 결함 발견률이 가장 높았다고 보고합니다. 그보다 큰 PR은 리뷰어의 집중력이 떨어져 “LGTM”으로 넘어가기 쉬우므로, 숫자 자체보다 PR을 작게 나누는 습관이 핵심입니다.
Q4: 자동화 도구는?
A:
- clang-tidy
- cppcheck
- SonarQube
- Coverity
Q5: 코드 리뷰 문화는?
A:
- 건설적인 피드백
- 코드를 공격하지 말고 개선 제안
- 배우는 기회로 활용
Q6: 리뷰 체크리스트를 만들려면?
A:
- 팀의 과거 버그 분석
- 자주 발생하는 실수 정리
- 코딩 스타일 가이드 포함
같이 보면 좋은 글
- C++ RAII: 생성자에서 획득하고 소멸자에서 해제하는 자원 관리 클래스 만들기
- C++ const 정확성
- C++ 스마트 포인터 | 3일 동안 찾지 못한 순환 참조 버그 해결법
- C++ 정적 분석 도구 통합: Clang-Tidy와 Cppcheck로 코드 퀄리티 강제하기
- 배열과 연결 리스트