[Libreoffice] va_start without va_end

[Julien Nabet]

Le 04/08/2011 10:33, Caolán McNamara a écrit :
> On Wed, 2011-08-03 at 23:56 +0200, Julien Nabet wrote:
>> Hello,
>>
>> In svl/source/items/itemset.cxx b/svl/source/items/itemset.cxx, I
>> noticed the use of va_start without va_end.
>> I read it could create undefined behaviour, so I propose this simple patch.
>>
> ...
>> +        va_end(pArgs);
>>        }
>>    }
>>
>> If it's ok, i can commit and push it on master.
> The va_end is tucked away hidden inside InitializeRanges_Impl in
> svl/source/items/nranges.cxx
>
> This looks rather ugly, it would be nicer to get the va_start and va_end
> closer together, e.g. move the va_end out of InitializeRanges_Impl and
> put it close to the two va_starts
Hello,

I had took a look at the called function but not at the "called called" 
function. Sorry for having missed this.
So I did what you proposed, commited and pushed the patch on master.
I created a tracker for new check on cppcheck about va_start not in 
pairs with va_end (ticket 2959)

I found other cases, this time I tried to find the called called...

1)
  /libs-gui/i18npool/source/calendar/
calendar_gregorian.cxx     62 va_start(ap, pat);
59 static void debug_cal_msg(const char *pat, ...)
      60 {
      61     va_list ap;
      62     va_start(ap, pat);
      63     vfprintf(stderr, pat, ap);
      64 }
I read that vfprintf didn't call automatically va_end 
(http://www.cplusplus.com/reference/clibrary/cstdio/vfprintf/)
so I would add a va_end there

2)
/libs-core/sfx2/source/menu/
mnumgr.cxx     444 va_start(pArgs, pArg1);
439 sal_uInt16 SfxPopupMenuManager::Execute( const Point& rPoint, 
Window* pWindow, const SfxPoolItem *pArg1, ... )
     440 {
     441     DBG_MEMTEST();
     442
     443     va_list pArgs;
     444     va_start(pArgs, pArg1);
     445
     446     return (Execute( rPoint, pWindow, pArgs, pArg1 ));
     447 }

which seems to call in the same file  :
    422 sal_uInt16 SfxPopupMenuManager::Execute( const Point& rPoint, 
Window* pWindow, va_list pArgs, const SfxPoolItem *pArg1 )
     423 {
     424     DBG_MEMTEST();
     425
     426     PopupMenu* pPopMenu = ( (PopupMenu*)GetMenu()->GetSVMenu() );
     427     pPopMenu->SetSelectHdl( LINK( this, SfxPopupMenuManager, 
SelectHdl ) );
     428     sal_uInt16 nId = pPopMenu->Execute( pWindow, rPoint );
     429     pPopMenu->SetSelectHdl( Link() );
     430
     431     if ( nId )
     432         GetBindings().GetDispatcher()->_Execute( nId, 
SFX_CALLMODE_RECORD, pArgs, pArg1 );
     433
     434     return nId;
     435 }

which seems to call in the file 
/libs-core/sfx2/source/control/dispatch.cxx :
const SfxPoolItem* SfxDispatcher::_Execute
    1381 (
    1382     sal_uInt16          nSlot,     // the Id of the executing 
function
    1383     SfxCallMode         eCall,     // SFX_CALLMODE_SYNCRHON, 
..._ASYNCHRON or
    1384                                    //..._SLOT
    1385     va_list             pVarArgs,  // Parameter list from the 
2nd parameter
    1386     const SfxPoolItem*  pArg1      // First parameter
    1387 )
    1388
    1389 /*  [Description]
    1390
    1391     Method to excecute a <SfxSlot>s over the Slot-Id.
    1392
    1393     [Return value]
    1394
    1395     const SfxPoolItem*      Pointer to the SfxPoolItem valid to 
the next run
    1396                             though the Message-Loop, which 
contains the return
    1397                             value.
    1398
    1399                             Or a NULL-Pointer, when the 
function was not
    1400                             executed (for example canceled by 
the user).
    1401 */
    1402
    1403 {
    1404     if ( IsLocked(nSlot) )
    1405         return 0;
    1406
    1407     SfxShell *pShell = 0;
    1408     const SfxSlot *pSlot = 0;
    1409     if ( GetShellAndSlot_Impl( nSlot, &pShell, &pSlot, sal_False,
    1410                                
SFX_CALLMODE_MODAL==(eCall&SFX_CALLMODE_MODAL) ) )
    1411     {
    1412        SfxAllItemSet aSet( pShell->GetPool() );
    1413
    1414        for ( const SfxPoolItem *pArg = pArg1;
    1415              pArg;
    1416              pArg = va_arg( pVarArgs, const SfxPoolItem* ) )
    1417            MappedPut_Impl( aSet, *pArg );
    1418
    1419        SfxRequest aReq( nSlot, eCall, aSet );
    1420        _Execute( *pShell, *pSlot, aReq, eCall );
    1421        return aReq.GetReturnValue();
    1422     }
    1423     return 0;
    1424 }

so I would add a va_end in SfxPopupMenuManager::Execute

3)
there are some cases (above all in win32 parts that I can't compile to 
check patch) like this one :
/components/setup_native/source/win32/customactions/reg64/
reg64.cxx     77 va_start( args, pFormat );
inline void OutputDebugStringFormat( const wchar_t* pFormat, ... )
      73 {
      74     wchar_t    buffer[1024];
      75     va_list args;
      76
      77     va_start( args, pFormat );
      78     StringCchVPrintf( buffer, sizeof(buffer), pFormat, args );
      79     OutputDebugString( buffer );
      80 }
I found nothing which tells if StringCchVPrintf calls automatically 
va_end or not.
Since this function seems derived from Vprintf, I would add a va_end too.

Julien