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

[Andrey Savchenko <bircoph@altlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 3905 bytes --]

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.

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]