<table><tr><td style="">ossi requested changes to this revision.<br />ossi 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/D11236">View Revision</a></tr></table><br /><div><div><p>yay, i finally have "some" time for this. ^^</p>
<p>it took me a while to get a coherent picture of the problem and solution, because your description unnecessarily dwells on the irrelevant cases of ptrace restrictions, but omits the crucial fact that this is about tracers that are not descendant processes of drkonqi, specifically kdevelop launched via kdeinit. see also comment on DefaultDebuggerLauncher in the complementary change. please make sure to point that out in the commit messages.</p>
<p>the socket code seemed like an unnecessary complication at first, but then i realized that launching via kdeinit makes it impossible to just talk to the child via stdout/stderr or at least pass a file descriptor of a socketpair end. that's something that could be actually addressed in the klauncher api, because it's possible to pass open fds to other processes over sockets.</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/D11236#inline-100741">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kcrash.cpp:655</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">//if the process was started directly, use waitpid(), as it's a child...</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span style="color: #aa4000">while</span> <span class="p">(</span><span class="n">waitpid</span><span class="p">(</span><span style="color: #aa2211">-</span><span style="color: #601200">1</span><span class="p">,</span> <span style="color: #aa4000">nullptr</span><span class="p">,</span> <span style="color: #601200">0</span><span class="p">)</span> <span style="color: #aa2211">!=</span> <span class="n">pid</span><span class="p">)</span> <span class="p">{}</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="p">}</span> <span style="color: #aa4000">else</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">you need to cover that branch as well.</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/D11236#inline-100732">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kcrash.cpp:668</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">// create socket path to transfer ptrace scope and open connection</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">socketpath</span> <span style="color: #aa2211">=</span> <span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"%1/kcrash_%2"</span><span class="p">).</span><span class="n">arg</span><span class="p">(</span><span class="n">QDir</span><span style="color: #aa2211">::</span><span class="n">tempPath</span><span class="p">()).</span><span class="n">arg</span><span class="p">(</span><span class="n">pid</span><span class="p">).</span><span class="n">toLocal8Bit</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">sockfd</span> <span style="color: #aa2211">=</span> <span class="n">openDrKonqiSocket</span><span class="p">(</span><span class="n">socketpath</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">use RuntimeLocation, as the kdeinit-related code (at start of setCrashHandler()) 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/D11236#inline-100727">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kcrash.cpp:870</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">bind</span><span class="p">(</span><span class="n">sockfd</span><span class="p">,</span> <span class="p">(</span><span style="color: #aa4000">struct</span> <span class="n">sockaddr</span> <span style="color: #aa2211">*</span><span class="p">)</span><span style="color: #aa2211">&</span><span class="n">drkonqi_server</span><span class="p">,</span> <span style="color: #aa4000">sizeof</span><span class="p">(</span><span class="n">drkonqi_server</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="n">perror</span><span class="p">(</span><span style="color: #766510">"Warning: bind() for communication with DrKonqi failed"</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">you need to unlink first, otherwise stale sockets will throw you off.</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/D11236#inline-100728">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kcrash.cpp:884</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">fd_set</span> <span class="n">set</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">FD_ZERO</span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">set</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">why don't you use poll()? this code is linux-specific anyway, so you can rely on posix functions from the previous decade.</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/D11236#inline-100730">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kcrash.cpp:904</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">clsockfd</span> <span style="color: #aa2211">=</span> <span class="n">accept</span><span class="p">(</span><span class="n">sockfd</span><span class="p">,</span> <span class="p">(</span><span style="color: #aa4000">struct</span> <span class="n">sockaddr</span> <span style="color: #aa2211">*</span><span class="p">)</span><span style="color: #aa2211">&</span><span class="n">drkonqi_client</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">cllength</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">while</span> <span class="p">(</span><span class="n">clsockfd</span> <span style="color: #aa2211">==</span> <span style="color: #aa2211">-</span><span style="color: #601200">1</span> <span style="color: #aa2211">&&</span> <span class="p">(</span><span class="n">errno</span> <span style="color: #aa2211">==</span> <span class="n">EINTR</span> <span style="color: #aa2211">||</span> <span class="n">errno</span> <span style="color: #aa2211">==</span> <span class="n">EAGAIN</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 class="n">clsockfd</span> <span style="color: #aa2211"><</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;">EAGAIN is a workaround for some unspecified broken non-linux systems, so you can drop that 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/D11236#inline-100731">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kcrash.cpp:915</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">ucred</span><span class="p">.</span><span class="n">pid</span> <span style="color: #aa2211">!=</span> <span class="n">pid</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">perror</span><span class="p">(</span><span style="color: #766510">"Warning: peer pid does not match DrKonqi pid"</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 style="color: #aa2211">-</span><span style="color: #601200">1</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">that's bogus use of perror()</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R285 KCrash</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11236">https://phabricator.kde.org/D11236</a></div></div><br /><div><strong>To: </strong>croick, Frameworks, ossi<br /><strong>Cc: </strong>dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, ngraham, bruns<br /></div>