Re: [PATCH] kernel-shark: Provide parsing for quotation marks in Record command line

["Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx>]

On 28.08.19 г. 14:30 ч., Steven Rostedt wrote:
> On Wed, 28 Aug 2019 09:25:33 -0400
> Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> > On Tue, 27 Aug 2019 16:14:18 +0300
> > "Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:
> >
> > > +	cmdTmp = _commandLineEdit.text();
> > > +	if (!cmdTmp.contains('\'') && !cmdTmp.contains('\"')) {
> > > +		/* Split all command line arguments. */
> > > +		argv << cmdTmp.split(" ", opt);
> > > +	} else {
> > > +		int iOpenQuots, iCloseQuots, size = cmdTmp.size();
> > > +		int iSingleQuots = (cmdTmp.count('\'') == 2) ? cmdTmp.indexOf('\'') : size;
> > > +		int iDoubleQuots = (cmdTmp.count('\"') == 2) ? cmdTmp.indexOf('\"') : size;
> > > +
> > > +		if (iSingleQuots < iDoubleQuots) {
> > > +			iOpenQuots = iSingleQuots;
> > > +			iCloseQuots = cmdTmp.lastIndexOf('\'') + 1;
> > > +		} else if (iDoubleQuots < iSingleQuots) {
> > > +			iOpenQuots = iDoubleQuots;
> > > +			iCloseQuots = cmdTmp.lastIndexOf('\"') + 1;
> > > +		} else {
> > > +			emit print("\nERROR: Unable to parse the command.");
> > > +			return {};
> > > +		}
> > > +
> > > +		/* Split the arguments. */
> > > +		argv << cmdTmp.left(iOpenQuots).split(" ", opt);
> > > +
> > > +		/* Everything in between the quotation marks goes in one piece. */
> > > +		argv << cmdTmp.mid(iOpenQuots, iCloseQuots - iOpenQuots);
> > > +
> > > +		/* Split the rest of the arguments. */
> > > +		argv << cmdTmp.right(size - iCloseQuots).split(" ", opt);
> > > +	}
> >
> > This is where I hate C++, because it makes simple things so
> > complicated ;-)
> >
> > What we need to do is simply:
> >
> > 	char *str = _commandLineEdit.text();
> > 	char *last_word = str;
> > 	char quote = 0;
> > 	int i;
> >
> > 	// remove front and end spaces
> > 	while (isspace(*str))
> > 		str++;
> > 	i = strlen(str);
> > 	while (i > 0 && isspace(str[i-1])
> > 		i--;
> > 	str[i] = 0;
>
> Oops, need to have:
>
> 	last_word = &str[i];
>
> here.
>
> -- Steve
>
> > 	for (i = 0; str[i]; i++) {
> > 		if (isspace(str[i]) && !quote) {
> > 			str[i++] = 0;
> > 			argv << last_word;
> > 			while (isspace(str[i]))
> > 				i++;
> > 			last_word = &str[i];
> > 		}
> > 		switch(str[i]) {
> > 		case '\\':
> > 			i++;
> > 			break;
> > 		case '\'':
> > 		case '"':
> > 			if (!quote)
> > 				quote = str[i];
> > 			else if (quote == str[i])
> > 				quote = 0;
> > 			break;
> > 		}
> > 	}
> > 	argv << last_word;
> >
> > Note, the above may be buggy, I didn't test it.

Hi Steve,

I understand very well your feelings ;)
However, I also really dislike eclectic programming stiles. Since this 
is part of the C++/Qt code, I would prefer to stick to the C++/Qt way of 
doing things.

Here is a version of your code that is more C++/Qt-ish:


	QString::SplitBehavior opt = QString::SkipEmptyParts;
	QChar quote = 0;
	int i, progress = 0, size;

	cmd.remove(QChar('\\'));
	size = cmd.count();
	auto lamMid = [&] () {return cmd.mid(progress, i - progress);};
	for (i = 0; i < size; ++i) {
		if (cmd[i] == '\'' || cmd[i] == '"') {
			if (quote.isNull()) {
				args << lamMid().split(" ", opt);
				quote = cmd[i++];
				progress = i;
			} else if (quote == cmd[i]) {
				args << lamMid();
				quote = 0;
				progress = ++i;
			}
		}
	}

	args << cmd.right(size - progress).split(" ", opt);

If you are OK with this I will send a new version of the patch.

Thanks!
Yordan



> > -- Steve