Re: [Libreoffice] va_start without va_end

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