<table><tr><td style="">pino added a comment.
</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/D17816">View Revision</a></tr></table><br /><div><div><p>general notes:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">NULL -> nullptr</li>
<li class="remarkup-list-item">there is not just glibc</li>
<li class="remarkup-list-item">the changes to <tt style="background: #ebebeb; font-size: 13px;">file_unix.cpp</tt> seem unrelated to you patch now, so better split them in an own patch</li>
<li class="remarkup-list-item">use <tt style="background: #ebebeb; font-size: 13px;">constData()</tt> instead of <tt style="background: #ebebeb; font-size: 13px;">data()</tt> every time the data needed is read-only</li>
</ul></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/D17816#inline-98263">View Inline</a><span style="color: #4b4d51; font-weight: bold;">config-kiocore.h.cmake:8</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="err">/*</span> <span class="err">Defined</span> <span class="err">if</span> <span class="err">system</span> <span class="err">has</span> <span class="err">extended</span> <span class="err">file</span> <span class="err">attributes</span> <span class="err">support.</span> <span class="err">*/</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">#cmakedefine01 HAVE_SYS_XATTR_H</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;">HAVE_SYS_XATTR_H</tt> is available here as side-effect of using the FindACL.cmake module.<br />
Better explicitly look for <tt style="background: #ebebeb; font-size: 13px;">sys/xattr.h</tt>, like <tt style="background: #ebebeb; font-size: 13px;">src/ioslaves/file/CMakeLists.txt</tt> does.</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/D17816#inline-98264">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:27-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: #304a96">#if HAVE_SYS_XATTR_H</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#if defined(Q_OS_LINUX) || defined(__GLIBC__) || defined(Q_OS_MAC)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#include</span> <span class="cpf"><sys/xattr.h></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#include</span> <span class="cpf"><sys/extattr.h></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#endif</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#endif</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this <tt style="background: #ebebeb; font-size: 13px;">#include</tt> block has a slightly messy logic:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">if <tt style="background: #ebebeb; font-size: 13px;">sys/attr.h</tt> is available, just include it directly with no other checks</li>
<li class="remarkup-list-item"><tt style="background: #ebebeb; font-size: 13px;">sys/extattr.h</tt> needs its own cmake check, including it if found</li>
</ul></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/D17816#inline-98265">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2191-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; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#if !(HAVE_SYS_XATTR_H)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <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 style="color: #304a96">#endif</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">if <tt style="background: #ebebeb; font-size: 13px;">HAVE_SYS_XATTR_H</tt> is not defined, the first instruction in <tt style="background: #ebebeb; font-size: 13px;">copyXattrs</tt> will be a return, and some compilers may warn/error out because the rest of the function is unreachable; better stub out the whole function instead</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/D17816#inline-98262">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2195</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">long</span> <span class="n">listlen</span><span class="p">,</span> <span class="n">valuelen</span><span class="p">,</span> <span class="n">destlen</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">QByteArray</span> <span class="n">sourcearray</span> <span style="color: #aa2211">=</span> <span class="n">source</span><span class="p">.</span><span class="n">path</span><span class="p">().</span><span class="n">toLocal8Bit</span><span class="p">().</span><span class="n">data</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 style="color: #aa4000">char</span> <span style="color: #aa2211">*</span><span class="n">xattrsrc</span> <span style="color: #aa2211">=</span> <span class="n">sourcearray</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;"><ul class="remarkup-list">
<li class="remarkup-list-item">you don't need to call <tt style="background: #ebebeb; font-size: 13px;">data()</tt> here, the return value of <tt style="background: #ebebeb; font-size: 13px;">toLocal8Bit()</tt> is already a QByteArray</li>
<li class="remarkup-list-item"><tt style="background: #ebebeb; font-size: 13px;">toLocal8Bit()</tt> is the wrong function here: please use <tt style="background: #ebebeb; font-size: 13px;">QFile::encodeName()</tt>, which does the right job for QString -> local filesystem paths</li>
</ul></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/D17816#inline-98266">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2201</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: #304a96">#elif defined(Q_OS_MAC)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">listlen</span> <span style="color: #aa2211">=</span> <span class="n">listxattr</span><span class="p">(</span><span class="n">xattrsrc</span><span class="p">,</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">NULL -> nullptr</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/D17816#inline-98267">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2203</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: #304a96">#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">listlen</span> <span style="color: #aa2211">=</span> <span class="n">extattr_list_file</span><span class="p">(</span><span class="n">xattr_src</span><span class="p">,</span> <span class="n">EXTATTR_NAMESPACE_USER</span><span class="p">,</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#endif</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">NULL -> nullptr</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/D17816#inline-98270">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2206</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 class="n">listlen</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="n">qCWarning</span><span class="p">(</span><span class="n">KIO_COPYJOB_DEBUG</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Glibc failed to extract xattr."</span><span class="p">;</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><span class="n">listlen</span> <span style="color: #aa2211">==</span> <span style="color: #601200">0</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;">there is not just glibc; also, better check for <tt style="background: #ebebeb; font-size: 13px;">errno</tt> as <tt style="background: #ebebeb; font-size: 13px;">ENOTSUP</tt>, because that means the source filesystem does not support xattrs, and thus you can directly skip the rest of the function (as it will not work anyway)</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/D17816#inline-98268">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2227</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: #304a96">#elif defined(Q_OS_MAC)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span class="n">valuelen</span> <span style="color: #aa2211">=</span> <span class="n">listxattr</span><span class="p">(</span><span class="n">xattrsrc</span><span class="p">,</span> <span class="n">xattrkey</span><span class="p">.</span><span class="n">data</span><span class="p">(),</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">NULL -> nullptr</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/D17816#inline-98269">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2229</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: #304a96">#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span class="n">valuelen</span> <span style="color: #aa2211">=</span> <span class="n">extattr_get_file</span><span class="p">(</span><span class="n">xattrsrc</span><span class="p">,</span> <span class="n">EXTATTR_NAMESPACE_USER</span><span class="p">,</span> <span class="n">xattrkey</span><span class="p">.</span><span class="n">data</span><span class="p">(),</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#endif</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">NULL -> nullptr</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/D17816#inline-98275">View Inline</a><span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:2257-2259</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 class="n">destlen</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="n">qCWarning</span><span class="p">(</span><span class="n">KIO_COPYJOB_DEBUG</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Glibc failed to write xattr on a file."</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;">as noted above: checking that <tt style="background: #ebebeb; font-size: 13px;">errno</tt> is <tt style="background: #ebebeb; font-size: 13px;">ENOTSUP</tt> means that the destination filesystem does not support xattrs, so there is little point trying to set the other attributes</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/D17816#inline-98276">View Inline</a><span style="color: #4b4d51; font-weight: bold;">file_unix.cpp:42</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">#if HAVE_SYS_XATTR_H</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#if defined(Q_OS_LINUX) || defined(__GLIBC__) || defined(Q_OS_MAC)</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #304a96">#include</span> <span class="cpf"><sys/xattr.h></span><span style="color: #304a96"></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;">HAVE_SYS_XATTR_H</tt> from <tt style="background: #ebebeb; font-size: 13px;">config-kioslave-file.h</tt> can be used 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/D17816#inline-98277">View Inline</a><span style="color: #4b4d51; font-weight: bold;">file_unix.cpp:44</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">#include</span> <span class="cpf"><sys/xattr.h></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#elif defined(Q_OS_FREEBSD) || defined(Q_OS_NETBSD)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#include</span> <span class="cpf"><sys/extattr.h></span><span style="color: #304a96"></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">better check for <tt style="background: #ebebeb; font-size: 13px;">sys/extattr.h</tt> instead</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/D17816">https://phabricator.kde.org/D17816</a></div></div><br /><div><strong>To: </strong>cochise, dfaure<br /><strong>Cc: </strong>pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, michaelh, bruns<br /></div>