<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111911/">http://git.reviewboard.kde.org/r/111911/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">A global KDE::open: why not, might make windows porting easier.
But for stat/QT_STAT, the best solution would be to port to QFileInfo.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111911/diff/1/?file=176375#file176375line2393" style="color: black; font-weight: bold; text-decoration: underline;">kioslave/ftp/ftp.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Ftp::copy( const QUrl &src, const QUrl &dest, int permissions, KIO::JobFlags flags )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2390</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">bool</span> <span class="n">bSrcExists</span> <span class="o">=</span> <span class="p">(</span><span class="n"><span class="hl">KDE</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">stat</span></span><span class="p"><span class="hl">(</span></span><span class="hl"> </span><span class="n">sCopyFile</span><span class="p">,</span> <span class="o">&</span><span class="n">buff</span> <span class="p">)</span> <span class="o">!=</span> <span class="o">-</span><span class="mi">1</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2392</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">bool</span> <span class="n">bSrcExists</span> <span class="o">=</span> <span class="p">(</span><span class="n"><span class="hl">QT_STAT</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">QFile</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">encodeName</span></span><span class="p"><span class="hl">(</span></span><span class="n">sCopyFile</span><span class="p"><span class="hl">).</span></span><span class="n"><span class="hl">constData</span></span><span class="p"><span class="hl">()</span>,</span> <span class="o">&</span><span class="n">buff</span><span class="p">)</span> <span class="o">!=</span> <span class="o">-</span><span class="mi">1</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This isn't unix-only code, it should be cross platform. So please use:
QFileInfo info(...);
bSrcExists = info.exists();
S_ISDIR becomes info.isDir();</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111911/diff/1/?file=176375#file176375line2407" style="color: black; font-weight: bold; text-decoration: underline;">kioslave/ftp/ftp.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Ftp::copy( const QUrl &src, const QUrl &dest, int permissions, KIO::JobFlags flags )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2404</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">iCopyFile</span> <span class="o">=</span> <span class="n"><span class="hl">KDE</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">open</span></span><span class="p">(</span> <span class="n">sCopyFile</span><span class="p">,</span> <span class="n">O_RDONLY</span> <span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2406</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">iCopyFile</span> <span class="o">=</span> <span class="n"><span class="hl">QT_OPEN</span></span><span class="p">(</span> <span class="n">sCopyFile</span><span class="p">,</span> <span class="n">O_RDONLY</span> <span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This seems more tricky to make portable. I'm OK if you leave it as an exercise to the (Windows) reader.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111911/diff/1/?file=176375#file176375line2515" style="color: black; font-weight: bold; text-decoration: underline;">kioslave/ftp/ftp.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Ftp::StatusCode Ftp::ftpCopyGet(int& iError, int& iCopyFile, const QString &sCopyFile,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2512</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// If rename fails, try removing the destination first if it exists.</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2514</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// If rename fails, try removing the destination first if it exists.</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Funny, the author didn't know that KDE::rename() would overwrite (on unix).
Well, this makes the porting easy.
(The trap is that porting from KDE::rename to QFile::rename changes the behavior when the destination already exists; but this particular code is ready for that)</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111911/diff/1/?file=176375#file176375line2540" style="color: black; font-weight: bold; text-decoration: underline;">kioslave/ftp/ftp.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Ftp::StatusCode Ftp::ftpCopyGet(int& iError, int& iCopyFile, const QString &sCopyFile,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2537</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">KDE</span></span><span class="o">::</span><span class="n">utime</span><span class="p">(</span><span class="n">sCopyFile</span><span class="p">,</span> <span class="o">&</span><span class="n">utbuf</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">2539</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="o">::</span><span class="n">uti<span class="hl">me</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">QFile</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">encodeNa</span>me</span><span class="p">(</span><span class="n">sCopyFile</span><span class="p"><span class="hl">).</span></span><span class="n"><span class="hl">constData</span></span><span class="p"><span class="hl">()</span>,</span> <span class="o">&</span><span class="n">utbuf</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Right, we're missing a portable utime (`touch`). This will do for now.
Actually, looking at sqlite3, it simply says "use utime if available, otherwise use utimes". Seems the porting to Windows will be easy then.</pre>
</div>
<br />
<p>- David</p>
<br />
<p>On August 6th, 2013, 4:11 p.m. UTC, Vishesh Handa wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Vishesh Handa.</div>
<p style="color: grey;"><i>Updated Aug. 6, 2013, 4:11 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">A couple of issues -
1. KDE::open/stat/etc take a QString and convert it to a char* via QFile::encodeName(str).constData(). Qt obviously does not have methods to do so. Instead of me doing it manually for each call to QT_OPEN/QT_STAT/etc, would be it okay for me to declare local functions called KDE::stat/open?
Something along the lines of -
namespace KDE {
int open(const QString& filePath, ...) {
return QT_OPEN(QFile::encodeName(filePath).constData()), ...);
}
}
2. The kioslave uses KDE::utime to set the utime of file. I've used ::utime, but that obviously won't work on non-unix platforms. What is the correct solution?
One option is to add utime in qplatformdefs.h, but that is non trivial since Qt seems to support about 104 different qplatformdefs and therefore all of them will have to be updated.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Doesn't even compile right now. With (1) it will start to compile.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kioslave/ftp/ftp.cpp <span style="color: grey">(a0da54b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111911/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>