<table><tr><td style="">tmarshall added inline comments.
</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><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-147544">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">copyjob.cpp:1119</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">That sounds racy; the subjob might not be finished by the time the main job is finished, if you just "start and forget".</p>

<p style="padding: 0; margin: 8px;">I cannot accept this commit with an exec() in here. The easy and modern way to make this async is just a connect and a lambda (which contains the rest of the code here).</p>

<p style="padding: 0; margin: 8px;">I have to wonder though: do we have a fast way to determine whether we actually need the additional subjob? (to avoid making this slower on systems -- or files -- without xattr)</p>

<p style="padding: 0; margin: 8px;">Hmm, or a much bigger architectural question: why doesn't kio_file copy the xattr as part of FileProtocol::copy(), as it already does with e.g. permissions and ACLs, instead of doing this in a separate job?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please help me understand for a moment. I'm not the original author of this patch and am trying to bring it to completion. I just need to be caught up to speed. In reading this piece of code over, I wasn't entirely sure of the author's intent. It seems like he wants to create a new job which copies the xattrs and then run it asynchronously. Why is the exec call bad? What does a lambda do that the exec call does not?</p>

<p style="padding: 0; margin: 8px;">In terms of determining when the job is actually required, one could test to see if the file has xattrs or indeed if the system has xattrs support. We could surround the invokation of the xattrs copy job with an <tt style="background: #ebebeb; font-size: 13px;">#ifdef HAVE_SYS_XATTR_H</tt>, for example.</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>tmarshall, dfaure, chinmoyr, bruns, Frameworks, cochise<br /><strong>Cc: </strong>tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, GB_2, michaelh<br /></div>