[apparmor] [patch 06/18] parser: replace reverse iterator [resend]

[Steve Beattie]

On Fri, Jan 24, 2014 at 02:40:59AM -0800, John Johansen wrote:
> On 01/16/2014 02:06 PM, Steve Beattie wrote:
> > As suggested by Seth Arnold, we can use string::find_last_not_of()
> > instead of using C++'s hideous reverse iterators.
> > 
> hrmmm honestly I don't think it is much different but

Well, my griping comment about C++'s reverse iterator was due to
str.erase() accepting only an iterator type, not a reverse_iterator,
necessitating the base() call, as well as the fact that the base
iterator, when dereferenced, points at the element preceding it,
hence the a priori decrement of the iterator before passing to
str.erase(). So a different solution with fewer hidden land mines that
captures the intent a little more explicitly is an improvement to me.

> Acked-by: John Johansen <john.johansen at canonical.com>

Thanks for all the reviews!

> > ===================================================================
> > --- a/parser/parser_variable.c
> > +++ b/parser/parser_variable.c
> > @@ -137,11 +137,11 @@ void free_var_string(struct var_string *
> >  
> >  static void trim_trailing_slash(std::string& str)
> >  {
> > -	for (std::string::reverse_iterator rit = str.rbegin();
> > -			rit != str.rend() && *rit == '/'; ++rit) {
> > -		/* yuck, reverse_iterators are ugly */
> > -		str.erase(--rit.base());
> > -	}
> > +	std::size_t found = str.find_last_not_of('/');
> > +	if (found != std::string::npos)
> > +		str.erase(found + 1);
> > +	else
> > +		str.clear(); // str is all '/'
> >  }

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140124/a34d9231/attachment.pgp>