<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120839/">https://git.reviewboard.kde.org/r/120839/</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Aside the minor WS issues, this patch looks correct and <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">required</em>.
Actually the present code looks extremely weird - the automounting is <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">only</em> processed if automounting unknown devices is disabled itfp??</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Together w/ the odd manipulation of the "lastSeen" state, ideally someone familiar with the code should make a statement on what was attempted with this apparent nonsense (maybe automounting was broken "intentionally"?)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If nobody does (or does veto), you've a +1 from here.</p></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="https://git.reviewboard.kde.org/r/120839/diff/1/?file=322442#file322442line76" style="color: black; font-weight: bold; text-decoration: underline;">solid-device-automounter/kded/DeviceAutomounter.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; ">DeviceAutomounter::automountDevice(Solid::Device &dev, AutomounterSettings::AutomountType type)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">76</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AutomounterSettings</span><span class="o">::</span><span class="n">setDeviceLastSeenMounted</span><span class="p">(</span><span class="n">dev</span><span class="p">.</span><span class="n">udi</span><span class="p">(),</span> <span class="n">sa</span><span class="o">-></span><span class="n">isAccessible</span><span class="p">());</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">76</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AutomounterSettings</span><span class="o">::</span><span class="n">setDeviceLastSeenMounted</span><span class="p">(</span><span class="n">dev</span><span class="p">.</span><span class="n">udi</span><span class="p">(),</span> <span class="n">sa</span><span class="o">-></span><span class="n">isAccessible</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">unrelated to this patch, but i'd like to raise:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"wtf does the function that apparently performs an automount impact the "lastSeen" state before?" sa->isAccessible() should be false right now, so doesn't this entirely defeat the idea of "lastSeen"?</p></pre>
</div>
</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="https://git.reviewboard.kde.org/r/120839/diff/1/?file=322442#file322442line79" style="color: black; font-weight: bold; text-decoration: underline;">solid-device-automounter/kded/DeviceAutomounter.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; ">DeviceAutomounter::automountDevice(Solid::Device &dev, AutomounterSettings::AutomountType type)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">79</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k"><span class="hl">if</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="o"><span class="hl">!</span></span><span class="n">AutomounterSettings</span><span class="o">::</span><span class="n"><span class="hl">a</span>utomount<span class="hl">Unknown</span>Device<span class="hl">s</span></span><span class="p"><span class="hl">(</span>))</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">79</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k"><span class="hl">if</span></span><span class="p"><span class="hl">(</span></span><span class="n">AutomounterSettings</span><span class="o">::</span><span class="n"><span class="hl">shouldA</span>utomountDevice</span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">dev</span></span><span class="p"><span class="hl">.</span></span><span class="n"><span class="hl">udi</span></span><span class="p"><span class="hl">(),</span></span><span class="hl"> </span><span class="n"><span class="hl">type</span></span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">while at it, you could inject a space between keyword and brace</p></pre>
</div>
</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="https://git.reviewboard.kde.org/r/120839/diff/1/?file=322443#file322443line78" style="color: black; font-weight: bold; text-decoration: underline;">solid-device-automounter/lib/AutomounterSettings.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; ">AutomounterSettings::shouldAutomountDevice(const QString &udi, AutomountType type)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">77</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">shouldAutomount</span> <span class="o">=</span> <span class="n">deviceAutomount</span> <span class="o">||</span> <span class="p">(</span><span class="n">enabled</span> <span class="o">&&</span> <span class="n">typeCondition</span> <span class="o"><span class="hl">&&</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">known</span></span><span class="hl"> </span><span class="o"><span class="hl">||</span></span><span class="hl"> </span><span class="n"><span class="hl">lastSeenMounted</span></span><span class="p"><span class="hl">));</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">78</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">shouldAutomount</span> <span class="o">=</span> <span class="n">deviceAutomount</span> <span class="o">||</span> <span class="p">(</span><span class="n">enabled</span> <span class="o">&&</span> <span class="n">typeCondition</span><span class="ew"> </span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">trailing WS</p></pre>
</div>
</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="https://git.reviewboard.kde.org/r/120839/diff/1/?file=322443#file322443line80" style="color: black; font-weight: bold; text-decoration: underline;">solid-device-automounter/lib/AutomounterSettings.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; ">AutomounterSettings::shouldAutomountDevice(const QString &udi, AutomountType type)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">80</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew"> </span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">trailing WS</p></pre>
</div>
</div>
<br />
<p>- Thomas Lübking</p>
<br />
<p>On Oktober 28th, 2014, 11:43 vorm. UTC, Frank Schütte wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for kdelibs, Solid and Christoph Feck.</div>
<div>By Frank Schütte.</div>
<p style="color: grey;"><i>Updated Okt. 28, 2014, 11:43 vorm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=243046">243046</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=261376">261376</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-runtime
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hi Christoph,
solid-device-automounter fails to automount unknown devices, even if it is checked,
because in kded/DeviceAutomounter.cpp it fails to evaluate shouldAutomountDevice.
So this patch corrects the logic to at first evaluate shouldAutomountDevice.
Inside this evaluation automountUnknownDevices is evaluated correctly.
Please review this patch. Christoph Feck urged me to submit this patch.
It took me quite a while to figure it out, though.
Bye,
Frank</p></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>solid-device-automounter/kded/DeviceAutomounter.cpp <span style="color: grey">(14b4e87)</span></li>
<li>solid-device-automounter/lib/AutomounterSettings.cpp <span style="color: grey">(2b3e6be)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120839/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>