[Kde-pim] ebn fixes: pass-by-value function args

[Ingo Klöcker]

On Tuesday 05 June 2007 20:42, Bernhard Breinbauer wrote:
> Attached is a patch that (hopefully) fixes the pass-by-value issues
> found by krazy.
> Please review and commit, if it's ok.

Thanks a bunch for this huge patch. In principal the patch looks good, 
but there are a few caveats:
a) The signature changes must not break method-overloading, i.e. the 
signature changes must be applied to all occurrences of the methods in 
the whole class hierarchy. Otherwise those changes will break 
polymorphism.

b) If the signature of a slot is changed the signature of the signal 
this slot is connected to must also be changed.


This is just one example regarding caveat b)
> -void KMFolderMgr::slotRenameDone( QString, bool success )
> +void KMFolderMgr::slotRenameDone( const QString&, bool success )

This slot is connected to the signal 
  void renameDone( QString newName, bool success );
of KMail::RenameJob, but your patch does not seem to change the 
signature of this signal. Please also change the signature of the 
signal.


I did not have a closer look at all the other slots, but I'm convinced 
that there are loads of signals whose signature also needs to be 
changed.


Then I noticed the following:
> -void FolderDialogGeneralTab::slotChangeIcon( QString icon ) // can't 
use a const-ref here, due to KIconButton's signal
> +void FolderDialogGeneralTab::slotChangeIcon( const QString &icon ) // 
can't use a const-ref here, due to KIconButton's signal

The comment is no longer valid because KIconButton's signal has already 
been fixed. Please remove the comment additionally to changing the 
signature.


Before the patch is applied it must be made sure that those changes 
break neither polymorphism or signal-slot connections because both 
breakages lead to hard-to-find bugs.


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20070610/c714a425/attachment.sig>
-------------- next part --------------
_______________________________________________
kde-pim mailing list
kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/