svn commit: r315773 - head/sbin/devd

[Ian Lepore]

On Thu, 2017-03-23 at 19:58 +0100, Roman Divacky wrote:
> On Thu, Mar 23, 2017 at 02:36:51AM +0000, Warner Losh wrote:
> > 
> > Author: imp
> > Date: Thu Mar 23 02:36:51 2017
> > New Revision: 315773
> > URL: https://svnweb.freebsd.org/changeset/base/315773
> > 
> > Log:
> >   Implement quote escaping. String values may now contain " if you
> >   it is preceded by \.
> >   
> >   foo="I \"like\" C++"
> >   
> >   gives the value 'I "like" C++' to the variable 'foo'. If a
> > character
> >   other than " follows the \, both the \ and that character are
> > passed
> >   through.
> >   
> >   Differential Revision: https://reviews.freebsd.org/D6286
> >   Sponsored by: Netflix
> > 
> > Modified:
> >   head/sbin/devd/devd.cc
> >   head/sbin/devd/devd.hh
> > 
> > Modified: head/sbin/devd/devd.cc
> > ===================================================================
> > ===========
> > --- head/sbin/devd/devd.cc	Thu Mar 23 02:33:27 2017	(
> > r315772)
> > +++ head/sbin/devd/devd.cc	Thu Mar 23 02:36:51 2017	(
> > r315773)
> > @@ -411,6 +411,32 @@ var_list::is_set(const string &var) cons
> >  	return (_vars.find(var) != _vars.end());
> >  }
> >  
> > +/** fix_value
> > + *
> > + * Removes quoted characters that have made it this far. \" are
> > + * converted to ". For all other characters, both \ and following
> > + * character. So the string 'fre\:\"' is translated to 'fred\:"'.
> > + */
> > +const std::string &
> > +var_list::fix_value(const std::string &val) const
> > +{
> > +	char *tmp, *dst;
> > +	const char *src;
> > +	std::string *rv;
> > +
> > +	dst = tmp = new char[val.length()];
> > +	src = val.c_str();
> > +	while (*src) {
> > +		if (*src == '\\' && src[1] == '"')
> > +			src++;
> > +		else
> > +			*dst++ = *src++;
> > +	}
> > +	rv = new string(tmp);
> > +	delete tmp;
> > +	return *rv;
> > +}
> Can the temporary char[] be stack allocated? Also, when returning a
> reference to
> a heap allocated string who is responsible for freeing it? Perhaps
> you can just
> return std::string and let the compiler optimize it?
> 
> Roman
> 

Returning a reference to a new'd string is not good.

IMO, the implementation of the function should take into account the
idea that embedded escaped quotes are rare.  The function should be
optimized for readability and the common case of simply copying the
string unmodified, something like:


std::string
var_list::fix_value(const std::string &val) const
{
	std::string rv(val);
	std::string::size_type pos(0);

	while ((pos = rv.find("\\\"", pos)) != rv.npos) {
		rv.erase(pos, 1);
	}
	return (rv);
}

-- Ian