<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112040/">http://git.reviewboard.kde.org/r/112040/</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;">Looks good, altough I'd suggest a tweak, see below.</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="http://git.reviewboard.kde.org/r/112040/diff/1/?file=178366#file178366line152" style="color: black; font-weight: bold; text-decoration: underline;">src/core-impl/collections/umscollection/UmsCollection.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; ">UmsCollectionFactory::identifySolidDevice( const QString &udi ) const</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">152</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// skip the encrypted volume itself, we only want to know about decrypted device</span></pre></td>
</tr>
<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">153</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">device</span><span class="p">.</span><span class="n">is</span><span class="o"><</span><span class="n">Solid</span><span class="o">::</span><span class="n">StorageVolume</span><span class="o">></span><span class="p">()</span> <span class="p">)</span></pre></td>
</tr>
<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">154</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">{</span></pre></td>
</tr>
<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">155</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">Solid</span><span class="o">::</span><span class="n">StorageVolume</span> <span class="o">*</span><span class="n">sv</span> <span class="o">=</span> <span class="n">device</span><span class="p">.</span><span class="n">as</span><span class="o"><</span><span class="n">Solid</span><span class="o">::</span><span class="n">StorageVolume</span><span class="o">></span><span class="p">();</span></pre></td>
</tr>
<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">156</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">sv</span> <span class="o">&&</span> <span class="n">sv</span><span class="o">-></span><span class="n">usage</span><span class="p">()</span> <span class="o">==</span> <span class="n">Solid</span><span class="o">::</span><span class="n">StorageVolume</span><span class="o">::</span><span class="n">Encrypted</span> <span class="p">)</span></pre></td>
</tr>
<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">157</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="nb">false</span><span class="p">;</span></pre></td>
</tr>
<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">158</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This makes sense, altough looking at other UsageType enum values we want really the FileSystem one and no other. So please convert the logic from "not encypted" to "only FileSystem". Code-wise, it is best to create a switch()/case with all the variants and no default: label to get notified on new enum values by the compiler in future.</pre>
</div>
<br />
<p>- Matěj</p>
<br />
<p>On August 12th, 2013, 8:44 p.m. CEST, Frank Meerkoetter wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok.</div>
<div>By Frank Meerkoetter.</div>
<p style="color: grey;"><i>Updated Aug. 12, 2013, 8:44 p.m.</i></p>
<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;">When I plug an encrypted USB-stick amarok is creating two UMS collections for it. Both have the same mount point.
$ solid-hardware list details
[...]
udi = '/org/freedesktop/UDisks/devices/dm_2d5'
parent = '/org/freedesktop/UDisks/devices/sdd1' (string)
vendor = '' (string)
product = '' (string)
description = '29.9 GiB Removable Media' (string)
Block.major = 252 (0xfc) (int)
Block.minor = 5 (0x5) (int)
Block.device = '/dev/dm-5' (string)
StorageAccess.accessible = true (bool)
StorageAccess.filePath = '/media/62a745fa-6350-4ee5-ba37-0462dfa3530f' (string)
StorageAccess.ignored = false (bool)
StorageVolume.ignored = false (bool)
StorageVolume.usage = 'FileSystem' (0x2) (enum)
StorageVolume.fsType = 'ext4' (string)
StorageVolume.label = '' (string)
StorageVolume.uuid = '62a745fa-6350-4ee5-ba37-0462dfa3530f' (string)
StorageVolume.size = 32125222912 (0x77ad00000) (qulonglong)
[...]
udi = '/org/freedesktop/UDisks/devices/sdd1'
parent = '/org/freedesktop/UDisks/devices/sdd' (string)
vendor = 'JetFlash' (string)
product = 'Transcend 32GB' (string)
description = '29.9 GiB Encrypted Container' (string)
Block.major = 8 (0x8) (int)
Block.minor = 49 (0x31) (int)
Block.device = '/dev/sdd1' (string)
StorageAccess.accessible = true (bool)
StorageAccess.filePath = '/media/62a745fa-6350-4ee5-ba37-0462dfa3530f' (string)
StorageAccess.ignored = false (bool)
StorageVolume.ignored = false (bool)
StorageVolume.usage = 'Encrypted' (0x5) (enum)
StorageVolume.fsType = 'crypto_LUKS' (string)
StorageVolume.label = '' (string)
StorageVolume.uuid = '1a38165b-2eee-41d0-acd1-6d34032f47fd' (string)
StorageVolume.size = 32127320064 (0x77af00000) (qulonglong)
This patch is filtering out the storage volume where the usage field is set to "Encrypted" (as opposed to "Filesystem").
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">I have tested plugin an USB-stick containing an dmcrypt/luks encrypted ext4fs. I also tested with an USB-stick that was not encrypted.</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>src/core-impl/collections/umscollection/UmsCollection.cpp <span style="color: grey">(028966e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112040/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>