Question
이번엔 제가 얼마전에 저질렀던 실수에 관한 문제입니다. :-|아래 함수는 dn을 입력받아 이 값을 key로 pfx를 찾고 만약 pfx가 존재한다면 pfx + dn을 uidn으로 리턴해주는 함수입니다. ((원래 코드를 간단하게 수정한 것으로 전화 번호에 지역번호를 붙여주는 함수입니다.
예: 7791000 -> (02)7791000))
이 getUIDigit() 함수에서 문제점을 찾아보세요.
//string findPrefix(const string& dn);
cp::ErrorCode getUIDigit(const char* dn, string& uidn)
{
string pfx = findPrefix(dn);
if (pfx.empty()) {
uidn = dn;
}
else {
uidn = pfx;
uidn += dn;
}
return cp::SUCCESS;
}
Answer
위의 문제만 보고 못 찾으신 분들은 아래 코드를 보면 금방 생각이 나실 겁니다. ((swap 함수를 사용하는 "create a temporary and swap" idiom을 사용하면 this와 비교하는 부분은 없어도 됩니다.관련 링크: Exception-Safe Class Design, Part 1: Copy Assignment))
Widet& operator=(const Widget& other)
{
if (this == &other) return *this;
...
}
바로 other 인자가 this인지 확인하는 코드입니다. 위의 함수도 이와 마찬가지로 다음과 같이 dn과 uidn 인자에 같은 객체를 넘기면 문제가 발생합니다.string dn(...);
cp::ErrorCode ret = getUIDigit(dn.c_str(), dn);
이 경우 uidn = pfx; 라인에서 dn이 가리키고 있던 영역은 invalidate됩니다. 접근 가능하다 하더라도 어떤 값이 들어 있을지는 모르는 일이죠. 제 경우엔 가장 안 좋은 문제가 발생했었습니다. 즉, 어떤 경우엔 제대로 동작하는 것처럼 보였던 것이죠. :-o이런 문제를 만들지 않으려면 체크 리스트에 어떤 항목을 추가해야 할까요? 제가 생각해 본 체크 포인트는 다음과 같습니다.
두 개 이상의 인자가 같은 타입(혹은 같은 객체를 가리킬 수 있는 타입)의 pointer나 reference 인 경우, 예를 들어 int*, int* 이거나 int*, int&, 혹은 const char*, string&, 그리고 이 중 적어도 하나가 writable인 경우, 인자들간의 관계를 확인하여 같은 instance더라도 문제가 발생하지 않는지 확인한다. ((하지만 다음과 같은 template function이라면 체크는 더욱 어려워지겠죠?
template <class T, class U> void foo(T t, U u);))
가장 쉬운 해결 방법은 read-only 인자들을 함수 내부에서 copy하여 사용하는 것입니다. 예를 들어 문제의 코드는 다음과 같이 수정이 가능합니다.
cp::ErrorCode getUIDigit(const char* orig_dn, string& uidn)
{
string dn(orig_dn);
... // 이하 이전 코드와 동일
}
인자의 copy가 비싼 경우에는 문제가 있으므로 copy assignment의 방법과 같이 실제 instance가 같은 것인지를 검사하여 case-by-case로 해결해야 할 것 같네요. ;-)
Comments
Post a Comment