<table><tr><td style="">ossi requested changes to this revision.<br />ossi added inline comments.<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/D11235">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/D11235#inline-100746">View Inline</a><span style="color: #4b4d51; font-weight: bold;">debuggerlaunchers.cpp:57</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">pid</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">queryPtrace</span><span class="p">(</span><span class="n">pid</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">m_monitor</span><span style="color: #aa2211">-></span><span class="n">startMonitoring</span><span class="p">(</span><span class="n">pid</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">according to my reading of the documentation (<a href="https://www.kernel.org/doc/Documentation/security/Yama.txt" class="remarkup-link" target="_blank" rel="noreferrer">https://www.kernel.org/doc/Documentation/security/Yama.txt</a>) and the kernel source, this is unnecessary in this branch, as descendants of the assigned ptracer have the right as well (that's why the "normal" backtrace creation works without your patch), and not even detaching breaks the chain.</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/D11235#inline-100742">View Inline</a><span style="color: #4b4d51; font-weight: bold;">queryptrace.cpp:67</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">select</span><span class="p">(</span><span class="n">sfd</span> <span style="color: #aa2211">+</span> <span style="color: #601200">1</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">set</span><span class="p">,</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">timeout</span><span class="p">)</span> <span style="color: #aa2211">></span> <span style="color: #601200">0</span> <span style="color: #aa2211">&&</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">read</span><span class="p">(</span><span class="n">sfd</span><span class="p">,</span> <span class="n">msg</span><span class="p">,</span> <span style="color: #601200">3</span><span class="p">)</span> <span style="color: #aa2211">></span> <span style="color: #601200">0</span> <span style="color: #aa2211">&&</span> <span class="n">strcmp</span><span class="p">(</span><span class="n">msg</span><span class="p">,</span> <span style="color: #766510">"OK"</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></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">use poll() here, too.</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/D11235#inline-100737">View Inline</a><span style="color: #4b4d51; font-weight: bold;">queryptrace.cpp:68</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">select</span><span class="p">(</span><span class="n">sfd</span> <span style="color: #aa2211">+</span> <span style="color: #601200">1</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">set</span><span class="p">,</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #304a96">NULL</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">timeout</span><span class="p">)</span> <span style="color: #aa2211">></span> <span style="color: #601200">0</span> <span style="color: #aa2211">&&</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">read</span><span class="p">(</span><span class="n">sfd</span><span class="p">,</span> <span class="n">msg</span><span class="p">,</span> <span style="color: #601200">3</span><span class="p">)</span> <span style="color: #aa2211">></span> <span style="color: #601200">0</span> <span style="color: #aa2211">&&</span> <span class="n">strcmp</span><span class="p">(</span><span class="n">msg</span><span class="p">,</span> <span style="color: #766510">"OK"</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">qCInfo</span><span class="p">(</span><span class="n">DRKONQI_LOG</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"ptrace granted by debugged process"</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">the current version of the complementary patch just echoes back the pid, so this cannot possibly work.</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/D11235#inline-67476">View Inline</a><span style="color: #4b4d51; font-weight: bold;">croick</span> wrote in <span style="color: #4b4d51; font-weight: bold;">queryptrace.cpp:26</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">QTemporaryDir will not work, since the location is set in KCrash.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">the location is still bogus, though. see comment on the other diff.</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/D11235#inline-100743">View Inline</a><span style="color: #4b4d51; font-weight: bold;">queryptrace.h:22</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">/** In Linux try to make the process attach to the debugger */</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">queryPtrace</span><span class="p">(</span><span class="n">qint64</span> <span class="n">pid</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">no. "on linux, tell the process to allow the debugger to attach to 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/D11235#inline-100744">View Inline</a><span style="color: #4b4d51; font-weight: bold;">queryptrace.h:23</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">/** In Linux try to make the process attach to the debugger */</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">queryPtrace</span><span class="p">(</span><span class="n">qint64</span> <span class="n">pid</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">that's a rather misleading function name. setPtracer() would reflect the purpose and upstream naming.</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/D11235#inline-54675">View Inline</a><span style="color: #4b4d51; font-weight: bold;">croick</span> wrote in <span style="color: #4b4d51; font-weight: bold;">crashtest.cpp:138</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">That reasoning does not seem to be true. The output can still be read when started "normally".</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">only if kdeinit was started from the same console, which you cannot assume.</p>
<p style="padding: 0; margin: 8px;">also, this change wouldn't belong into this patch anyway.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R871 DrKonqi</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11235">https://phabricator.kde.org/D11235</a></div></div><br /><div><strong>To: </strong>croick, Plasma: Workspaces, Frameworks, ossi<br /><strong>Cc: </strong>ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, sukalyanbanga, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>