[ros-dev] Re: 18113:-reorderInsertXscendingOrder macro argument order and update uses

[Joseph Galbraith]

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