[Mesa-dev] [PATCH] r600/sb: cleanup if_conversion iterator to be legal C++

[Mathias Fröhlich]

Hi,

On Friday, 29 June 2018 04:49:42 CEST Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> The current code causes:
> /usr/include/c++/8/debug/safe_iterator.h:207:
> Error: attempt to copy from a singular iterator.
> 
> This is due to the iterators getting invalidated, fix the
> reverse iterator to use the return value from erase, and
> cast it properly.

Looks correct.

One thing inline below:

> 
> Cc: <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/drivers/r600/sb/sb_if_conversion.cpp | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/gallium/drivers/r600/sb/sb_if_conversion.cpp b/src/gallium/drivers/r600/sb/sb_if_conversion.cpp
> index 3f6431b80f5..5556531f145 100644
> --- a/src/gallium/drivers/r600/sb/sb_if_conversion.cpp
> +++ b/src/gallium/drivers/r600/sb/sb_if_conversion.cpp
> @@ -42,16 +42,13 @@ int if_conversion::run() {
>  	regions_vec &rv = sh.get_regions();
>  
>  	unsigned converted = 0;
> -
> -	for (regions_vec::reverse_iterator N, I = rv.rbegin(), E = rv.rend();
> -			I != E; I = N) {
> -		N = I; ++N;
> -
> +	for (regions_vec::reverse_iterator I = rv.rbegin(); I != rv.rend(); ) {
>  		region_node *r = *I;
>  		if (run_on(r)) {
> -			rv.erase(I.base() - 1);
> +			I = decltype(I){rv.erase(std::next(I).base())};


I don't exactly know for mesa, do we already generally and strictly
require c++11?

If c++11 is not strictly available, you may instead of the above write

I = regions_vec::reverse_iterator(rv.erase((++I).base()));

without loss.

In any case:

Reviewed-by: Mathias Fröhlich <mathias.froehlich at web.de>

best

Mathias

>  			++converted;
> -		}
> +		} else
> +			++I;
>  	}
>  	return 0;
>  }
>