From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on sa.local.altlinux.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=unavailable autolearn_force=no version=3.4.1 To: devel@lists.altlinux.org References: <20191211234857.GB17949@altlinux.org> <20191212095730.83787-1-darktemplar@altlinux.org> <20191212095730.83787-2-darktemplar@altlinux.org> <20191213003225.71e5de1963ffea37fcd4c140@altlinux.org> From: Aleksei Nikiforov Message-ID: <5618b388-7241-8ef8-cec1-3178568094e7@altlinux.org> Date: Fri, 13 Dec 2019 11:29:09 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20191213003225.71e5de1963ffea37fcd4c140@altlinux.org> Content-Type: text/plain; charset=koi8-r; format=flowed Content-Language: ru Content-Transfer-Encoding: 8bit Subject: Re: [devel] [PATCH for apt v2 01/21] Replace post-increments with pre-increments X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Dec 2019 08:29:14 -0000 Archived-At: List-Archive: List-Post: 13.12.2019 0:32, Andrey Savchenko пишет: > 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. > Thanks, I'll review this patch once more. A note: if compiler is smart, it takes time to do these heuristics, and thus this change would improve compile time in that case. Otherwise, this change may improve runtime performance. > Best regards, > Andrew Savchenko > > > _______________________________________________ > Devel mailing list > Devel@lists.altlinux.org > https://lists.altlinux.org/mailman/listinfo/devel >