D27965: [KPasswdServer] replace foreach with range/index-based for

David Faure noreply at phabricator.kde.org
Thu Mar 12 20:37:13 GMT 2020


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kpasswdserver.cpp:201
>  
>  bool KPasswdServer::hasPendingQuery(const QString &key, const KIO::AuthInfo &info)
>  {

I wonder why the whole method isn't const

> kpasswdserver.cpp:713
> +       if (current->info.realmValue == info.realmValue) {
> +          authList->erase(it);
>            authItem = current;

(this one is fine, there's a "break;" below)

> apol wrote in kpasswdserver.cpp:648
> Use erase.

This is not the way to use erase(). It invalidates the iterator...

The proper way is

  it = authList->erase(it);

and doing ++it at the end of the for loop, not in the 3rd part of the for() line, since we don't want ++it after it=erase(it).

> apol wrote in kpasswdserver.cpp:682
> Use erase.

same problem here.
add "} else { ++it; }"

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D27965

To: ahmadsamir, #frameworks, dfaure, meven
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200312/472fd2d5/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list