----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1004/#review1590 -----------------------------------------------------------
Basically none of the changes have effect. Please, next time double check if the zeroing of the variables is really necessary. /trunk/KDE/kdenetwork/kopete/kopete/chatwindow/chatview.cpp <http://reviewboard.kde.org/r/1004/#comment992> Not necessary either. The iterator is not used anywhere else. /trunk/KDE/kdenetwork/kopete/kopete/chatwindow/kopetechatwindow.cpp <http://reviewboard.kde.org/r/1004/#comment993> Not necessary, since backgroundFile will receive a new value in the line below /trunk/KDE/kdenetwork/kopete/kopete/chatwindow/kopetechatwindowstylemanager.cpp <http://reviewboard.kde.org/r/1004/#comment994> Not necessary, deletedStyle is a local var that gets out of scope right after the deletion /trunk/KDE/kdenetwork/kopete/kopete/config/accounts/kopeteaccountconfig.cpp <http://reviewboard.kde.org/r/1004/#comment995> Again, not necessary. lvi is a local var inside the for() loop, so after the breaking it will disappear /trunk/KDE/kdenetwork/kopete/kopete/config/appearance/layout/TokenDropTarget.cpp <http://reviewboard.kde.org/r/1004/#comment996> Not necessary. token is a pointer passed by value, so zeroing it makes no effect outside the function scope /trunk/KDE/kdenetwork/kopete/kopete/kopetewindow.cpp <http://reviewboard.kde.org/r/1004/#comment991> Not necessary. sbIcon is a local temporary var. /trunk/KDE/kdenetwork/kopete/kopete/kopetewindow.cpp <http://reviewboard.kde.org/r/1004/#comment990> Not necessary. sbIcon is a local temporary var /trunk/KDE/kdenetwork/kopete/kopete/statusmenu/kopetestatusgroupaction.cpp <http://reviewboard.kde.org/r/1004/#comment997> Zeroing a local var at the end of the function makes no sense. /trunk/KDE/kdenetwork/kopete/kopete/statusmenu/kopetestatusrootaction.cpp <http://reviewboard.kde.org/r/1004/#comment998> Zeroing a local var in the end of the function makes no sense /trunk/KDE/kdenetwork/kopete/libkopete/kopeteaccountmanager.cpp <http://reviewboard.kde.org/r/1004/#comment999> The variable is not used after its deletion, so no need to zero it /trunk/KDE/kdenetwork/kopete/libkopete/kopetechatsession.cpp <http://reviewboard.kde.org/r/1004/#comment1000> Zeroing a pointer variable passed as value has no effect /trunk/KDE/kdenetwork/kopete/libkopete/kopetecommandhandler.cpp <http://reviewboard.kde.org/r/1004/#comment1001> Zeroing a pointer variable passed as value has no effect /trunk/KDE/kdenetwork/kopete/libkopete/kopetecontact.cpp <http://reviewboard.kde.org/r/1004/#comment1002> The variable is not used past this point, so no need to zero it /trunk/KDE/kdenetwork/kopete/libkopete/kopeteidentitymanager.cpp <http://reviewboard.kde.org/r/1004/#comment1003> Zeroing a pointer variable passed as value has no effect /trunk/KDE/kdenetwork/kopete/libkopete/kopeteproperty.cpp <http://reviewboard.kde.org/r/1004/#comment1004> No need to set "d" as zero, as it will get a new value in the next line. /trunk/KDE/kdenetwork/kopete/libkopete/kopeteproperty.cpp <http://reviewboard.kde.org/r/1004/#comment1005> No need to set "d" to zero as we are in the destructor of the class. /trunk/KDE/kdenetwork/kopete/libkopete/kopetewalletmanager.cpp <http://reviewboard.kde.org/r/1004/#comment1006> No need to zero a local variable at the end of its scope /trunk/KDE/kdenetwork/kopete/libkopete/private/kopeteviewmanager.cpp <http://reviewboard.kde.org/r/1004/#comment1007> No need to zero a local variable at the end of its scope /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.cpp <http://reviewboard.kde.org/r/1004/#comment1008> This is not necessary either. /trunk/KDE/kdenetwork/kopete/libkopete/ui/kopetelistviewitem.cpp <http://reviewboard.kde.org/r/1004/#comment1009> This change doesn't make any sense. "c" has exactly the purpose to be used for deleting the objects from the list. - Gustavo On 2009-07-13 21:40:37, Lamarque Souza wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1004/ > ----------------------------------------------------------- > > (Updated 2009-07-13 21:40:37) > > > Review request for Kopete. > > > Summary > ------- > > Initialise deleted pointers to 0L. Throughout all Kopete source code pointers > are deleted and not initialized, grep -r 'delete ' $(find \( -name "*.cpp" > -o -name "*.h" \) ) | wc returns 1105 lines, I can do a rough estimation of > at least half of those lines do not reinitialize pointers. Althouth > reinitialing pointers are not always necessary in some cases it does is > necessary and by what I have seen there are such places in Kopete's source > code. Kopete::CommandHandler::slotExecFinished even passed one deleted > pointer to a function, which seems really wrong to me, it should delete the > pointer after calling the function, not before. To illustrate the problem > assume a is a class member pointer: > > delete a; // in one class method > > // In another class method: > if (a) // the result will the true even though a is deleted > do a->"something" // this can crash the application > > > Diffs > ----- > > /trunk/KDE/kdenetwork/kopete/kopete/chatwindow/chatview.cpp 993925 > /trunk/KDE/kdenetwork/kopete/kopete/chatwindow/kopetechatwindow.cpp 993925 > > /trunk/KDE/kdenetwork/kopete/kopete/chatwindow/kopetechatwindowstylemanager.cpp > 993925 > /trunk/KDE/kdenetwork/kopete/kopete/config/accounts/kopeteaccountconfig.cpp > 993925 > > /trunk/KDE/kdenetwork/kopete/kopete/config/appearance/layout/TokenDropTarget.cpp > 993925 > /trunk/KDE/kdenetwork/kopete/kopete/kopetewindow.cpp 993925 > /trunk/KDE/kdenetwork/kopete/kopete/statusmenu/kopetestatusgroupaction.cpp > 993925 > /trunk/KDE/kdenetwork/kopete/kopete/statusmenu/kopetestatusrootaction.cpp > 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/kopeteaccountmanager.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/kopetechatsession.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/kopetecommandhandler.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/kopetecontact.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/kopeteidentitymanager.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/kopeteproperty.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/kopetewalletmanager.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/private/kopeteviewmanager.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/ui/avatarselectorwidget.cpp 993925 > /trunk/KDE/kdenetwork/kopete/libkopete/ui/kopetelistviewitem.cpp 993925 > /trunk/KDE/kdenetwork/kopete/protocols/wlm/wlmchatmanager.cpp 993925 > /trunk/KDE/kdenetwork/kopete/protocols/wlm/wlmlibmsn.cpp 993925 > > Diff: http://reviewboard.kde.org/r/1004/diff > > > Testing > ------- > > > Thanks, > > Lamarque > > _______________________________________________ kopete-devel mailing list kopete-devel@kde.org https://mail.kde.org/mailman/listinfo/kopete-devel