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
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
