Comment 143 for bug 214366

Revision history for this message
In , Standard8 (standard8) wrote :

Comment on attachment 8390075
patch v6 - local folders

Review of attachment 8390075:
-----------------------------------------------------------------

::: mailnews/base/util/nsMsgDBFolder.cpp
@@ +3843,5 @@
>
> +nsresult
> +nsMsgDBFolder::ConfirmAutoFolderRename(nsIMsgWindow *msgWindow,
> + const nsString aOldName,
> + const nsString aNewName,

These should be const nsString &... to avoid unnecessary string copies.

@@ +3846,5 @@
> + const nsString aOldName,
> + const nsString aNewName,
> + bool *confirmed)
> +{
> + NS_ENSURE_ARG_POINTER(confirmed);

It is possible that confirmed never gets set, so you might then end up with a bad value.

Additionally I don't really see the point of returning nsresult here, when you're already warning for errors, and you treat a failure as not confirmed anyway.

So I think it would be simpler just to change this function to return the bool.