[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apparmor-dev
Subject:    Re: [apparmor] [patch 06/18] parser: replace reverse iterator [resend]
From:       Steve Beattie <steve () nxnw ! org>
Date:       2014-01-24 19:33:07
Message-ID: 20140124193307.GH8199 () nxnw ! org
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


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@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@ubuntu.com>
http://NxNW.org/~steve/

["signature.asc" (application/pgp-signature)]

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor


[prev in list] [next in list] [prev in thread] [next in thread]