Issue with QProcess and forward slashed arguments after upgrade

jfossati

Currently in the process of upgrading Qt from 5.12.x to 5.15.9 for my application. The previous call of QProcess was deprecated, and needed the additional QStringList() passed into the arguments parameter to work.

This previous code worked:

QString cmdPath = "\"" + system32Dir + "xcopy.exe\" ";
cmdPath += "/E ";
cmdPath += "/C "; 
//...so on with a few more flags

cmdPath+= QString("\"") + dirToCopy + "\" \"" + destDir + "\"";

QProcess * scriptProcess = new QProcess();
scriptProcess->start ( cmdPath );

QString stError = scriptProcess->readAllStandardError();

After upgrading to 5.15.9, we changed the start command to

scriptProcess->start( cmdPath, QStringList());

We now get an error that says there are an invalid number of arguments (likely from xcopy).

Other things that have been attempted include:

using setProgram() and setArguments()/setNativeArguments()
not using a pointer instance of QProcess
outputting the arguments passed from setArguments with arguments(), and those all seem correct.
including the flags as part of the cmdPath variable
creating the arguments variable as start( cmdPath, QStringList() << "/E" << "/C" ) etc
creating the arguments variable via QStringList args = { "/E", "/C" } etc

I'm assuming the issue comes from the flags that we pass, since the issue also occurs on another call that uses a forward slashed argument, and the copy works without the flags.
Any help would be appreciated

JonB

@jfossati said in Issue with QProcess and forward slashed arguments after upgrade:

scriptProcess->start( cmdPath, QStringList());

Well that's just wrong.

QString cmdPath = system32Dir + "xcopy.exe";
scriptProcess->start( cmdPath, { "/E", "/C", dirToCopy, destDir });

or similar. Build up a QStringList variable holding each argument if you want to/it's easier/more flexible. Note that we do not quote each argument, as they are separate Qt will do whatever necessary.

You should pass paths (dirToCopy, destDir) with \s, utilise QString QDir::toNativeSeparators() if it helps. Using /s in these is at your own risk, some will go wrong.

creating the arguments variable as start( cmdPath, QStringList() << "/E" << "/C" ) etc

creating the arguments variable via QStringList args = { "/E", "/C" } etc

Either of these should work. If not look at the exact arguments you are passing. Saying "etc." is no good.

jfossati

just ran it with the following line

testProcess->start( cmdPathY, QStringList() << "/E" << "/C" << "/Y" << "/R" << "/I" << "/F" << dirToCopy << destDir);

with the same result as building the QStringList variable separately

Christian Ehrlicher

@jfossati What's cmdPathY and what's the outut of QProcess (stdout, stderr) and the error code?

mchinand

In addition to looking at the stdout/stderr output, I would also look at what startProcess->program() and startProcess->arguments() return.

mchinand

Do either dirToCopy or destDir paths have spaces in them?

JonB

@mchinand
To the best of my knowledge any quoting required is deliberately left to QProcess::start(). I told the OP to remove any quoting of paths he was doing.

Christian Ehrlicher

@JonB said in Issue with QProcess and forward slashed arguments after upgrade:

required is deliberately left to QProcess::start()

Even QProcess has nothing to quote here since the single QStringList items are passed as single arguments without any quoting to the executable.

jfossati

the Y in cmdPathY was a typo. The stdErr said that xcopy was not getting the proper number of arguments, which I expected.

the ->program() and ->arguments() calls both returned what is expected, formatted properly.

Yes, destDir has spaces in it, but I tried both quoting and not quoting them to see if that would change anything, which it did not.

We've gone on for several days trying to debug with numerous different approaches. Eventually this seemed to fix the issue

scriptProcess->setCreateProcessArgumentsModifier([] (QProcess::CreateProcessArguments *args)
{
args->flags |= CREATE_NEW_CONSOLE;
args->startupInfo->dwFlags &= ~STARTF_USESTDHANDLES;
});

JonB

@jfossati
I have no idea why creating a new console and not inheriting standard handles would "fix" the issue. Would have thought it would just briefly flash a new console. Implication is that xcopy does not like running with no stdin. Still might depend on whether you have changed dirToCopy and destDir from Qt /s to native \s, but you don't say.

Also, whether it affects this or not, you must put a scriptProcess->waitForFinished() before scriptProcess->readAllStandardError();, if you are not you are lucky it worked (reported the right error).

jfossati

@JonB I saw this and figured I'd try it because I'd exhausted many other options.

Regarding the dirs we use a delimiter variable that's assigned to QString("\\") on windows to build out paths, so I don't think the issue comes from there, unless I'm misunderstanding your concern for which type of slash to use. We also use a waitForStarted waitForFinished and exitCode in order to check for stderrs

JonB

@jfossati
That stuff sounds good. It's up to you whether you want to pursue further, like I said your "fix" does not seem right/relevant to me (and I also said I would expect it to flash up a new console for a moment). I am happy to help, but don't have Qt under Windows.

If it were me, with all your waitForFinished()/readAllStandardOutput()/readAllStandardError() in place I would start from say (literally):

testProcess->start("xcopy", QStringList() << "/Y" << "C:\\rubbish" << "C:\\trash");

and build up from there....