Bug(ErrorChecker): Some bugs #5
Description
🚨 Critical Issues Identified in ErrorChecker.hpp by DeepSeek
During code review, several critical issues were identified in the ErrorChecker class that require immediate attention:
1. Broken Assignment Operators
// MISSING return statements!
ErrorChecker& operator=(const ErrorChecker& other) {
this->code = other.code;
this->msg = other.msg;
// ❌ MISSING: return *this;
};
ErrorChecker& operator=(const DWORD& other) {
this->code = other;
format();
// ❌ MISSING: return *this;
};Impact: Chain assignment (a = b = c;) will fail to compile.
2. Exception Safety Vulnerability
void format() {
PWSTR LocalAddress = NULL;
if (!FormatMessageW(/*...*/, (PWSTR)&LocalAddress, /*...*/)) {
SetLastError(code);
return; // ❌ Inconsistent state: code updated but msg unchanged
}
msg = LocalAddress; // ❌ Potential memory leak if bad_alloc thrown
LocalFree((HLOCAL)LocalAddress);
SetLastError(code);
}Issues:
- Memory leak if
msg = LocalAddressthrowsstd::bad_alloc - Inconsistent object state when formatting fails
3. Unnecessary String Conversion
void check() const {
throw w32oop::exceptions::win32_exception(
w32oop::util::str::converts::wstr_str(message()) // ❌ wstring→string
);
}Issue: Unnecessary conversion in Unicode project; exceptions should use wstring.
4. Missing Move Semantics
No move constructor or move assignment operator, missing optimization opportunities for wstring member.
🔧 Recommended Fixes
1. Fix Assignment Operators
ErrorChecker& operator=(const ErrorChecker& other) {
if (this != &other) {
code = other.code;
msg = other.msg;
}
return *this; // ✅ Add return
}2. RAII Resource Management
struct MessageBuffer {
PWSTR buffer = nullptr;
explicit MessageBuffer(PWSTR buf) : buffer(buf) {}
~MessageBuffer() { if (buffer) LocalFree((HLOCAL)buffer); }
// Delete copy operations
};3. Exception-Safe Format
void format() {
PWSTR rawBuffer = nullptr;
if (!FormatMessageW(...)) {
msg.clear(); // ✅ Consistent state
return;
}
MessageBuffer bufferGuard(rawBuffer);
msg = rawBuffer; // ✅ RAII ensures cleanup
}4. Add Move Operations
ErrorChecker(ErrorChecker&& other) noexcept;
ErrorChecker& operator=(ErrorChecker&& other) noexcept;📋 Additional Suggestions
- Add helper methods:
succeeded(),failed() - Add static utility methods
- Add comparison operators
Priority: High
Severity: Critical (compilation errors + resource leaks)