Nathan Woods wrote:
Richard Campbell writes:
Hate to throw my 2 cents in, but why use macros or goto statements at all? None of the demonstrated code actually needs a goto statement to work. Granted i've not seen the actual offensive code , but all examples here can be written without goto statements or macros. Why bother using either? At any rate, i'm inclined to agree that macros are a bad idea. Hiding a mess behind a preprocessor is considered bad coding practice.
While the previous examples can ge accomplished cleanly without gotos and/or macros, the rules can be different when the code is far more complicated. For instance, the handling of error conditions buried deep within multiple for loops and other statements. Take the following example:
BOOL NtFunc() { BOOL bSuccess = FALSE; LPVOID *pPointer = NULL; LPVOID *pAnotherPointer = NULL;
for (...) { if (ThisOperationCanFail()) goto cleanup;
pPointer = malloc(...); if (!pPointer) goto cleanup; while (...) { if (ThisOperationCanFailAlso()) goto cleanup; .... if (!pAnotherPointer) { pAnotherPointer = malloc(...); if (!pPointer) goto cleanup; if (YetAnotherCanFail()) goto cleanup; ... } } free(pPointer); pPointer = NULL;} bSuccess = TRUE; cleanup: if (pPointer) free(pPointer); if (pAnotherPointer) free(pAnotherPointer); return bSuccess; }
pPointer and pAnotherPointer are both pointers with different lifetimes; making things a bit messy. While it is possible to write such code without gotos or macros, it would involve checking and freeing those pointers from many different places. Using a cleanup label provides a convenient place to centrally locate all cleanup code.
Of course the real solution to this problem is the following simple C++ class (which I'm not really arguing we should use... I'm just throwing some gasoline on the flames :-)
template<class T> class NPagedPointer { public: inline NPagedPointer(T* p) : m_ptr(p) {} inline ~NPagedPointer() { if ( m_ptr ) ::ExFreePool(m_ptr); }
inline operator T*() { return m_ptr; } };
BOOL NtFunc() { NPagedPointer<PLUGH> pAnotherPointer;
for (...) { if (ThisOperationCanFail()) return FALSE;
NPagedPointer<XYZZY> pPointer = malloc(...); if ( ! pPointer ) return FALSE;
while (...) { if (ThisOperationCanFailAlso()) return FALSE;
....
if ( ! pAnotherPointer) { pAnotherPointer = malloc(...); if ( pAnotherPointer ) return FALSE; if (YetAnotherCanFail()) return FALSE; ... } } }
return TRUE; }
See, no cleanup code needed :-)
Of course, people don't like to use C++ in the kernel, and not everyone has agreed to learn C++ (anymore than they've agreed to learn Zimbobwean.) And it can be tricky to make sure the C++ compiler doesn't mess you up (like putting the generated template code in a paged section if you have one.)
Thanks,
Joseph