<table><tr><td style="">dfaure requested changes to this revision.<br />dfaure added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D23579">View Revision</a></tr></table><br /><div><div><p>Nice work!</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136514">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:33</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #aa4000">public</span><span style="color: #aa2211">:</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QUrl</span> <span class="n">url</span><span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">QString</span> <span style="color: #aa2211">&</span><span class="n">path</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this method should be const</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136516">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:50</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// works around the fact that kioslave would load the slave from the system</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// as first choice instead of the one from teh build dir.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qputenv</span><span class="p">(</span><span style="color: #766510">"QT_PLUGIN_PATH"</span><span class="p">,</span> <span class="n">QCoreApplication</span><span style="color: #aa2211">::</span><span class="n">applicationDirPath</span><span class="p">().</span><span class="n">toUtf8</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">typo: teh -> the</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136515">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:53</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// Run ftpd to talk t.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY</span><span class="p">(</span><span class="n">m_remoteDir</span><span class="p">.</span><span class="n">isValid</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">"talk t" ? missing 'o'?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136518">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:61</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY</span><span class="p">(</span><span class="n">m_daemonProc</span><span class="p">.</span><span class="n">waitForStarted</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY</span><span class="p">(</span><span class="n">m_daemonProc</span><span class="p">.</span><span class="n">state</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">Running</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// Wait for the daemon to print its port. That tells us both where it's listening</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QCOMPARE so we see the state if it fails</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136519">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:97</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">m_daemonProc</span><span class="p">.</span><span class="n">state</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">Running</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QCOMPARE</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136520">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:114</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">job</span><span style="color: #aa2211">-></span><span class="n">setUiDelegate</span><span class="p">(</span><span style="color: #aa4000">nullptr</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">exec</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QCOMPARE</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">data</span><span class="p">(),</span> <span class="n">data</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QVERIFY2(job->exec(), qUtf8Printable(job->errorString()));</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136521">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:167</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">job</span><span style="color: #aa2211">-></span><span class="n">setUiDelegate</span><span class="p">(</span><span style="color: #aa4000">nullptr</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QEXPECT_FAIL</span><span class="p">(</span><span style="color: #766510">""</span><span class="p">,</span> <span style="color: #766510">"Trying to write to an inacessible path fails."</span><span class="p">,</span> <span class="n">Continue</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY2</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">exec</span><span class="p">(),</span> <span class="n">qUtf8Printable</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">errorString</span><span class="p">()));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QEXPECT_FAIL is for known bugs.</p>
<p style="padding: 0; margin: 8px;">Here, what you want is</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">QVERIFY(!job->exec());</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136523">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:180</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">inaccessibleUrl</span><span class="p">.</span><span class="n">setPassword</span><span class="p">(</span><span style="color: #766510">"password"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">const</span> <span class="n">QString</span> <span class="n">remoteInaccesiblePath</span> <span style="color: #aa2211">=</span> <span class="n">m_remoteDir</span><span class="p">.</span><span class="n">path</span><span class="p">()</span> <span style="color: #aa2211">+</span> <span class="n">inaccessiblePath</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY</span><span class="p">(</span><span class="n">QFile</span><span style="color: #aa2211">::</span><span class="n">copy</span><span class="p">(</span><span class="n">QFINDTESTDATA</span><span class="p">(</span><span style="color: #766510">"ftp/testCopy1"</span><span class="p">),</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">missing a 's' in Inaccessible</p>
<p style="padding: 0; margin: 8px;">I'm confused whether this test is about inaccessible path or something around resuming.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136522">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:186</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">job</span><span style="color: #aa2211">-></span><span class="n">setUiDelegate</span><span class="p">(</span><span style="color: #aa4000">nullptr</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QEXPECT_FAIL</span><span class="p">(</span><span style="color: #766510">""</span><span class="p">,</span> <span style="color: #766510">"Trying to write with bad quot doesn't work."</span><span class="p">,</span> <span class="n">Continue</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY2</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">exec</span><span class="p">(),</span> <span class="n">qUtf8Printable</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">errorString</span><span class="p">()));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">what's bad quot?</p>
<p style="padding: 0; margin: 8px;">Is this test about a bug, or the job failing is what we want?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136524">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:206</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">job</span><span style="color: #aa2211">-></span><span class="n">setUiDelegate</span><span class="p">(</span><span style="color: #aa4000">nullptr</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVERIFY</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">exec</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QCOMPARE</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">error</span><span class="p">(),</span> <span style="color: #601200">0</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I should ask for a clazy rule about QVERIFY(job->exec()) :-)<br />
[same as above, QVERIFY2...]</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136525">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftptest.cpp:217</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">// FIXME: Actually BROKEN</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Now *that* is what QEXPECT_FAIL is about :-)</p>
<p style="padding: 0; margin: 8px;">You can re-enable the code, and use QEXPECT_FAIL above the failing QVERIFY or QCOMPARE.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136530">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:671</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">result</span><span class="p">.</span><span class="n">success</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">result</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't think we want to return on failure here.<br />
If the server doesn't support this command, we might as well try proceeding.<br />
That's why the old code actually ignored errors here.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136531">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:696</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">qCDebug</span><span class="p">(</span><span class="n">KIO_FTP</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Searching for pwd"</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span style="color: #aa4000"><span class="bright">if</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">!</span></span><span class="n">ftpSendCmd</span><span class="p">(</span><span class="n">QByteArrayLiteral</span><span class="p">(</span><span style="color: #766510">"PWD"</span><span class="p">))<span class="bright"></span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">||</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">m_iRespType</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">!=</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">2</span></span><span class="bright"></span><span class="p"><span class="bright">))</span></span><span class="bright"> </span><span class="p"><span class="bright">{</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span class="n"><span class="bright">result</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span class="n">ftpSendCmd</span><span class="p">(</span><span class="n">QByteArrayLiteral</span><span class="p">(</span><span style="color: #766510">"PWD"</span><span class="p">))<span class="bright">;</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">result</span><span class="p">.</span><span class="n">success</span> <span style="color: #aa2211">||</span> <span class="p">(</span><span class="n">m_iRespType</span> <span style="color: #aa2211">!=</span> <span style="color: #601200">2</span><span class="p">))</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">The reuse of the <tt style="background: #ebebeb; font-size: 13px;">result</tt> var irks me a bit.<br />
const vars rock :-)</p>
<p style="padding: 0; margin: 8px;">Would it be a bad idea to make Result implicitly convertible to bool, so you can keep doing this inside the if? <tt style="background: #ebebeb; font-size: 13px;"> if (!ftpSendCmd(QByteArrayLiteral("PWD")) || (m_iRespType != 2)) {</tt> would still work fine then.</p>
<p style="padding: 0; margin: 8px;">This would simplify all cases where we test for failures, but ignore the actual error message on failure (because we'll provide a more high-level one, like here). In other cases, we indeed need a result var.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136532">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:801</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"></span><span style="color: #aa4000"><span class="bright">if</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="n">ftpOpenConnection</span><span class="p">(<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">l</span>oginDefered</span><span class="p">)<span class="bright">)</span></span><span class="bright"> </span><span class="p"><span class="bright">{</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span style="color: #aa4000"><span class="bright">const</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">auto</span></span><span class="bright"> </span><span class="n"><span class="bright">result</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span class="n">ftpOpenConnection</span><span class="p">(<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">L</span>ogin<span class="bright">Mode</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="n">Defered</span><span class="p">)<span class="bright">;</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#pragma message "we ignore success of the new command its unclear if that is intentional"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">result</span><span class="p">.</span><span class="n">success</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Oh... it's a recursive call... wow, tricky.</p>
<p style="padding: 0; margin: 8px;">Yeah, that means this method returns false even if the nested method managed to login. Very confusing, and probably wrong...</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136533">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:828</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">Result</span><span style="color: #aa2211">::</span><span class="n">fail</span><span class="p">();</span> <span style="color: #74777d">// else we don't know what went wrong</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think this is all quite redundant.</p>
<p style="padding: 0; margin: 8px;">openConnection() sets m_bLoggedOn to true if it succeeds, and to false if it fails.<br />
So the ret val from openConnection and m_bLoggedOn are the same thing.<br />
It makes little sense to test both independently and then even have a third case of failure in this line.<br />
It's all the same, I suggest to simplify this.</p>
<p style="padding: 0; margin: 8px;">I think this can all be simplified to</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">if (!openResult.success) {
if (m_control) { // ...
closeConnection();
}
return openResult; // or Result::fail(ERR_CANNOT_LOGIN, m_host), if the error code from openResult is no good
}</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136534">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1120</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">errormessage</span> <span style="color: #aa2211">=</span> <span class="n">m_host</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">return</span> <span class="bright"></span><span style="color: #304a96"><span class="bright">false</span></span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="bright"></span><span class="n"><span class="bright">Result</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">fail</span></span><span class="bright"></span><span class="p"><span class="bright">()</span>;</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">why not use errorcode and errormessage here?<br />
or in fact just removing that line, so that it goes to the correct <tt style="background: #ebebeb; font-size: 13px;">return Result::fail(errorcode...)</tt> below.</p>
<p style="padding: 0; margin: 8px;">I believe this was a bug in the orig code, setting two local vars and then returning false is nonsense, the <tt style="background: #ebebeb; font-size: 13px;">return false</tt> shouldn't have been there.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136535">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1213</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">ftpFolder</span><span class="p">(</span><span class="n">src</span><span class="p">.</span><span class="n">left</span><span class="p">(</span><span class="n">pos</span> <span style="color: #aa2211">+</span> <span style="color: #601200">1</span><span class="p">),</span> <span style="color: #304a96">false</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">return</span> <span class="bright"></span><span style="color: #304a96"><span class="bright">false</span></span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="bright"></span><span class="n"><span class="bright">Result</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">fail</span></span><span class="bright"></span><span class="p"><span class="bright">()</span>;</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">ERR_CANNOT_ENTER_DIRECTORY, src</tt></p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136536">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1490</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span class="n"><span class="bright">Ftp</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="n">stat<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">linkURL</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">return</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "this used to return nothing and ignored the </span>stat<span class="bright"> what gives"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return<span class="bright"></span></span><span class="bright"> </span><span class="n"><span class="bright">FtpInternal</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">stat</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">linkURL</span></span><span class="bright"></span><span class="p"><span class="bright">)</span>;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">That's because it was a recursive call, so it left the callee in charge of emitting error/finished.<br />
Not doing it here again was actually correct.</p>
<p style="padding: 0; margin: 8px;">Now that error codes are returned, your change is correct, you can remove the pragma.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136537">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1602</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"> </span><span style="color: #74777d"><span class="bright">// Servers running with Turkish locale having problems converting 'i' letter to upper case.</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #74777d">// So we send correct upper case command as last resort.</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">ftpOpenCommand</span><span class="p">(</span><span style="color: #766510">"LIST -la"</span><span class="p">,</span> <span class="n">QString</span><span class="p">(),</span> <span style="color: #766510">'I'</span><span class="p">,</span> <span class="n">ERR_CANNOT_ENTER_DIRECTORY</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="bright"></span><span class="n"><span class="bright">result</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span class="n"><span class="bright">ftpOpenCommand</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #766510"><span class="bright">"list"</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">QString</span></span><span class="bright"></span><span class="p"><span class="bright">(),</span></span><span class="bright"> </span><span style="color: #766510"><span class="bright">'I'</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">KJob</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">NoError</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">What happened to those two lines of comments? Can I suggest to keep them?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136539">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1880</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">ftpOpenConnection</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">loginImplicit</span></span><span class="bright"></span><span class="p"><span class="bright">)</span>)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span style="color: #aa4000"><span class="bright">return</span></span><span class="bright"> </span><span class="n"><span class="bright">statusS</span>erver<span class="bright">E</span>rror<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">;</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">result</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">success</span></span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "was s</span>erver<span class="bright"> e</span>rror<span class="bright"> here"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">result</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Wasn't that just a way to propagate the error from the low-level method to this method? Your new mechanism replaces that, no?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136538">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1908</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">qCWarning</span><span class="p">(</span><span class="n">KIO_FTP</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Can't open for reading"</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span style="color: #aa4000"><span class="bright">return</span></span><span class="bright"> </span><span class="n"><span class="bright">statusS</span>erver<span class="bright">E</span>rror<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">;</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "was s</span>erver<span class="bright"> e</span>rror<span class="bright"> here"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">result</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Same as above: why do we still need this distinction? Is this enum useful?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136540">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:1991</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span> <span style="color: #aa4000">else</span> <span style="color: #aa4000">if</span> <span class="p">((<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">i</span>Error</span> <span style="color: #aa2211">=</span> <span class="n">WriteToFile</span><span class="p">(</span><span class="n">iCopyFile</span><span class="p">,</span> <span class="n">buffer</span><span class="p">,</span> <span class="n">n</span><span class="p">))</span> <span style="color: #aa2211">!=</span> <span style="color: #601200">0</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span style="color: #aa4000"><span class="bright">return</span></span><span class="bright"> </span><span class="n"><span class="bright">statusClientError</span></span><span class="bright"></span><span class="p"><span class="bright">;</span></span><span class="bright"> </span><span style="color: #74777d"><span class="bright">// client side error</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span> <span style="color: #aa4000">else</span> <span style="color: #aa4000">if</span> <span class="p">((<span class="bright"></span></span><span class="bright"></span><span class="n"><span class="bright">write</span>Error</span> <span style="color: #aa2211">=</span> <span class="n">WriteToFile</span><span class="p">(</span><span class="n">iCopyFile</span><span class="p">,</span> <span class="n">buffer</span><span class="p">,</span> <span class="n">n</span><span class="p">))</span> <span style="color: #aa2211">!=</span> <span style="color: #601200">0</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "we cannot construct the final error here because we do not know what the target fiel path is"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">Result</span><span style="color: #aa2211">::</span><span class="n">fail</span><span class="p">(</span><span class="n">writeError</span><span class="p">,</span> <span class="n">QString</span><span class="p">(),</span> <span class="n">StatusCode</span><span style="color: #aa2211">::</span><span class="n">ClientError</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yep. Pass sCopyFile as argument, when ftpCopyGet calls ftpGet, just like what happens with iCopyFile.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136541">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2032</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #74777d"> qCDebug(KIO_FTP) << "finished";</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span style="color: #74777d"> finished<span class="bright">(</span>);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> finished<span class="bright">2(Q_FUNC_INFO</span>);</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #74777d"> qCDebug(KIO_FTP) << "after finished";</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136542">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2085</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span style="color: #aa4000"><span class="bright">if</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">iE</span>rror<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span><span class="bright"> </span><span class="p"><span class="bright">{</span></span><span class="bright"> </span><span style="color: #74777d"><span class="bright">// can have only server side errs</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span class="n"><span class="bright">error</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">iError</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">url</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">path</span></span><span class="bright"></span><span class="p"><span class="bright">());</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="n">Ftp</span><span style="color: #aa2211">::</span><span class="n">StatusCode</span> <span class="n">Ftp</span><span style="color: #aa2211">::</span><span class="n">ftpPut</span><span class="p">(</span><span style="color: #aa4000">int</span> <span style="color: #aa2211">&</span><span class="n">iError</span><span class="p">,</span> <span style="color: #aa4000">int</span> <span class="n">iCopyFile</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QUrl</span> <span style="color: #aa2211">&</span><span class="n">dest_url</span><span class="p">,</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "ier</span>rror<span class="bright"> should be moot with results"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span class="n"><span class="bright">Result</span></span><span class="bright"> </span><span class="n"><span class="bright">FtpInternal</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">ftpPut</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span style="color: #aa4000"><span class="bright">int</span></span><span class="bright"> </span><span class="n"><span class="bright">iCopyFile</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">const</span></span><span class="bright"> </span><span class="n"><span class="bright">QUrl</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&</span></span><span class="bright"></span><span class="n"><span class="bright">dest_url</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">outdated warning?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136543">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2169</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">storResult</span><span class="p">.</span><span class="n">success</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#pragma message "re-constructing failure"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">Result</span><span style="color: #aa2211">::</span><span class="n">fail</span><span class="p">(</span><span class="n">storResult</span><span class="p">.</span><span class="n">error</span><span class="p">,</span> <span class="n">storResult</span><span class="p">.</span><span class="n">errorString</span><span class="p">,</span> <span class="n">StatusCode</span><span style="color: #aa2211">::</span><span class="n">ServerError</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">because of the useless enum? ;)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136544">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2193</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">result</span> <span style="color: #aa2211"><</span> <span style="color: #601200">0</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span class="n"><span class="bright">iError</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span class="n"><span class="bright">ERR_CANNOT_WRITE</span></span><span class="bright"></span><span class="p"><span class="bright">;</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "isnt this cannot read we fail to read the input file here no"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">writeError</span> <span style="color: #aa2211">=</span> <span class="n">ERR_CANNOT_WRITE</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">yeah, feel free to change it.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136545">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2341</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">m_iRespType</span> <span style="color: #aa2211">!=</span> <span style="color: #601200">2</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span style="color: #aa4000"><span class="bright">if</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">bR</span>eport<span class="bright">E</span>rror<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span><span class="bright"> </span><span class="p"><span class="bright">{</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span class="bright"> </span><span class="n"><span class="bright">e</span>rror<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">ERR_CANNOT_ENTER_DIRECTORY</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">path</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "r</span>eport<span class="bright">e</span>rror<span class="bright"> has been broken not sure what to do with it"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #74777d"><span class="bright">//</span> <span class="bright">if (bReportE</span>rror<span class="bright">) {</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Well, that's part of your refactoring. It was used to decide if a method should emit error on failure or just return false. It was part of the big mess that you're cleaning up. Kill bReportError completely.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136546">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2363</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Result</span> <span class="n">result</span> <span style="color: #aa2211">=</span> <span class="n">Result</span><span style="color: #aa2211">::</span><span class="n">pass</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">bSrcLocal</span> <span style="color: #aa2211">&&</span> <span style="color: #aa2211">!</span><span class="n">bDestLocal</span><span class="p">)</span> <span class="p">{</span> <span style="color: #74777d">// File -> Ftp</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">optimistic but useless initializations, all code paths set (or ignore) result.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136548">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2409</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #304a96">#else</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">return</span> <span style="color: #004012">ftpPut</span><span class="p">(</span><span class="n">iError</span><span class="p">,</span> <span class="n">iCopyFile</span><span class="p">,</span> <span class="n">url</span><span class="p">,</span> <span class="n">permissions</span><span class="p">,</span> <span class="n">flags</span> <span style="color: #aa2211">|</span> <span class="n">KIO</span><span style="color: #aa2211">::</span><span class="n">Resume</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #304a96">#endif</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">probably needs the same treatment, i.e. removing iError?</p>
<p style="padding: 0; margin: 8px;">(ifdef'ed out code)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136549">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2488</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">result</span> <span style="color: #aa2211">=</span> <span class="n">Result</span><span style="color: #aa2211">::</span><span class="n">fail</span><span class="p">(</span><span class="n">ERR_CANNOT_WRITE</span><span class="p">,</span> <span class="n">StatusCode</span><span style="color: #aa2211">::</span><span class="n">ClientError</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#pragma message "shouldnt we return here"</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">no, we keep going in order to delete the .part file</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136550">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2596</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">qCDebug</span><span class="p">(</span><span class="n">KIO_FTP</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"user canceled proxy authentication, or communication error."</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"><span class="bright"> </span><span class="n"><span class="bright">error</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">errorCode</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">m_proxyURL</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">host</span></span><span class="bright"></span><span class="p"><span class="bright">());</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright"></span><span style="color: #304a96"><span class="bright">#pragma message "problem this is a slot so we cant return a result up the call chain maybe do it like errno or something"</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">// error(errorCode, m_proxyURL.host());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yes, clearly. But the question is where to check it.</p>
<p style="padding: 0; margin: 8px;">Hmm, how does this method even get called, without an event loop? From <tt style="background: #ebebeb; font-size: 13px;">m_control->waitForReadyRead</tt>, maybe?</p>
<p style="padding: 0; margin: 8px;">The old code was broken here, it would for sure emit error+error or error+finished, since the main code had no idea that this slot emitted error already.</p>
<p style="padding: 0; margin: 8px;">Hmm. Shouldn't we do this connect *before* the waitForConnected in Ftp::synchronousConnectToHost? If a proxy exists, it will surely act at connection time?<br />
And then it becomes easier, the place where to check the errno-like member is just after the call to Ftp::synchronousConnectToHost...</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136551">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2706</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">finalize</span><span class="p">(</span><span class="n">d</span><span style="color: #aa2211">-></span><span class="n">openConnection</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This looks wrong. openConnection() should emit connected() or error(), but never finished().</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136552">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.cpp:2766</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qDebug</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span class="n">result</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">result</span><span class="p">.</span><span class="n">success</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">remember to clean up the qDebug()s before the final version</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136526">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.h:70</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #aa4000">struct</span> <span class="n">Result</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">bool</span> <span class="n">success</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">int</span> <span class="n">error</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think you can make all members const?</p>
<p style="padding: 0; margin: 8px;">(This will prevent bypassing the "factory methods"...)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136527">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.h:95</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #a0a000">private</span><span class="p">:</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Result</span><span class="p">(</span><span style="color: #aa4000">bool</span> <span class="n">s</span><span class="p">,</span> <span style="color: #aa4000">int</span> <span class="n">e</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QString</span> <span style="color: #aa2211">&</span><span class="n">errString</span><span class="p">,</span> <span class="n">StatusCode</span> <span class="n">code</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa2211">:</span> <span class="n">success</span><span class="p">(</span><span class="n">s</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This class is an aggregate, so you could probably remove this ctor and use <tt style="background: #ebebeb; font-size: 13px;">Result{...}</tt> syntax in the above three methods.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136528">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.h:156</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">void</span> <span class="n">finished</span><span class="p">()</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Does this method serve any purpose?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D23579#inline-136529">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ftp.h:340</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);"> <span style="color: #aa4000">void</span> <span style="color: #004012">ftpShortStatAnswer</span><span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">QString</span> <span style="color: #aa2211">&</span><span class="n">filename</span><span class="p">,</span> <span style="color: #aa4000">bool</span> <span class="n">isDir</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">void</span> <span style="color: #004012">ftpShortStatAnswer</span><span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">QString</span> <span style="color: #aa2211">&</span><span class="n">filename</span><span class="p">,</span> <span style="color: #aa4000">bool</span> <span class="n">isDir</span><span class="p">)<span class="bright"></span></span><span class="bright"> </span><span class="n"><span class="bright">Q_REQUIRED_RESULT</span></span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Q_REQUIRED_RESULT on a method returning void??</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D23579">https://phabricator.kde.org/D23579</a></div></div><br /><div><strong>To: </strong>sitter, dfaure<br /><strong>Cc: </strong>dfaure, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns<br /></div>