Re: [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