On Thu, 12 Dec 2019 12:57:10 +0300 Aleksei Nikiforov wrote: > Found via cppcheck: > (performance) Prefer prefix ++/-- operators for non-primitive types. [...] > diff --git a/apt/cmdline/apt-get.cc b/apt/cmdline/apt-get.cc > index a26c93c..3858752 100644 > --- a/apt/cmdline/apt-get.cc > +++ b/apt/cmdline/apt-get.cc > @@ -541,8 +541,7 @@ bool DownloadPackages(vector &URLLst) > pkgAcquire Fetcher(&Stat); > > // Load the requestd sources into the fetcher > - vector::const_iterator I = URLLst.begin(); > - for (; I != URLLst.end(); I++) > + for (auto I = URLLst.begin(); I != URLLst.end(); ++I) > new pkgAcqFile(&Fetcher,*I,"",0,*I,flNotDir(*I)); This one contains the unrelated change for the iterator initialization. If you really need this cosmetics, submit it as a separate patch. > @@ -551,7 +550,7 @@ bool DownloadPackages(vector &URLLst) > > // Print error messages > bool Failed = false; > - for (pkgAcquire::ItemIterator I = Fetcher.ItemsBegin(); I != Fetcher.ItemsEnd(); I++) > + for (auto I = Fetcher.ItemsBegin(); I != Fetcher.ItemsEnd(); ++I) Same here. > @@ -1721,8 +1720,7 @@ bool DoDSelectUpgrade(CommandLine &CmdL) > return false; > > // Install everything with the install flag set > - pkgCache::PkgIterator I = Cache->PkgBegin(); > - for (;I.end() != true; I++) > + for (auto I = Cache->PkgBegin(); not I.end(); ++I) And here. > @@ -1732,7 +1730,7 @@ bool DoDSelectUpgrade(CommandLine &CmdL) > > /* Now install their deps too, if we do this above then order of > the status file is significant for | groups */ > - for (I = Cache->PkgBegin();I.end() != true; I++) > + for (auto I = Cache->PkgBegin(); not I.end(); ++I) And here. > { > /* Install the package only if it is a new install, the autoupgrader > will deal with the rest */ > @@ -1741,7 +1739,7 @@ bool DoDSelectUpgrade(CommandLine &CmdL) > } > > // Apply erasures now, they override everything else. > - for (I = Cache->PkgBegin();I.end() != true; I++) > + for (auto I = Cache->PkgBegin(); not I.end(); ++I) And here. > @@ -1758,7 +1756,7 @@ bool DoDSelectUpgrade(CommandLine &CmdL) > // Hold back held packages. > if (_config->FindB("APT::Ignore-Hold",false) == false) > { > - for (pkgCache::PkgIterator I = Cache->PkgBegin(); I.end() == false; I++) > + for (auto I = Cache->PkgBegin(); not I.end(); ++I) And here. > diff --git a/apt/cmdline/apt-shell.cc b/apt/cmdline/apt-shell.cc > index 9582291..0aa5da5 100644 > --- a/apt/cmdline/apt-shell.cc > +++ b/apt/cmdline/apt-shell.cc > @@ -3034,7 +3034,7 @@ bool DoList(CommandLine &CmdL) > string status = "available"; > if (Pkg->CurrentVer != 0) status = "installed"; > if (Pkg->CurrentVer != 0) > - for (pkgCache::DepIterator D = Pkg.RevDependsList(); D.end() == false; D++) > + for (pkgCache::DepIterator D = Pkg.RevDependsList(); not D.end(); ++D) Same here. Though this D.end() related change looks as an absolutely useless cosmetics. > diff --git a/apt/methods/http.cc b/apt/methods/http.cc > index dcdd651..da3e646 100644 > --- a/apt/methods/http.cc > +++ b/apt/methods/http.cc > @@ -383,10 +383,10 @@ int ServerState::RunHeaders() > if (Debug == true) > clog << Data; > > - for (string::const_iterator I = Data.begin(); I < Data.end(); I++) > + for (string::const_iterator I = Data.begin(); I != Data.end(); ++I) This is the correct change. But yet again it is unrelated to the patch description, please submit Data.end() fix as a separate patch. Other changes LGTM. They may improve performance (and may not depending on how smart a compiler is), but are harmless otherwise. Best regards, Andrew Savchenko