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

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

On 28.08.19 г. 14:30 ч., Steven Rostedt wrote:
> On Wed, 28 Aug 2019 09:25:33 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> On Tue, 27 Aug 2019 16:14:18 +0300
>> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> 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
>