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.johan...@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
<sbeat...@ubuntu.com>
http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
