Re: [devel] [PATCH for apt v2 01/21] Replace post-increments with pre-increments

[Aleksei Nikiforov <darktemplar@altlinux.org>]

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<string> &URLLst)
>>      pkgAcquire Fetcher(&Stat);
>>   
>>      // Load the requestd sources into the fetcher
>> -   vector<string>::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<string> &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
>