Review Request 112040: Do not create duplicate UMS collections for encrypted volumes

[Frank Meerkoetter]

> On Aug. 12, 2013, 7:36 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/umscollection/UmsCollection.cpp, line 174
> > <http://git.reviewboard.kde.org/r/112040/diff/2/?file=178369#file178369line174>
> >
> >     code style nitpick: spaces around args

ack


> On Aug. 12, 2013, 7:36 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/umscollection/UmsCollection.cpp, lines 137-139
> > <http://git.reviewboard.kde.org/r/112040/diff/2/?file=178369#file178369line137>
> >
> >     nitpicky simplification suggestion: if you use the interface afterwards, there's no point in calling the ::is<>() method, just call the ::as<>() method an then check the pointer.
> >     
> >     Other extremely low-importance suggestion: we somehow prefer early exit than nesting, so something like
> >     if( !sv )
> >         retrun false;
> >     
> >     would save you one nesting, which we like.

true, less nesting is good


> On Aug. 12, 2013, 7:36 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/umscollection/UmsCollection.cpp, line 134
> > <http://git.reviewboard.kde.org/r/112040/diff/2/?file=178369#file178369line134>
> >
> >     style: spaces around arg (in declaration it is right)

ack


> On Aug. 12, 2013, 7:36 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/umscollection/UmsCollection.h, lines 90-91
> > <http://git.reviewboard.kde.org/r/112040/diff/2/?file=178368#file178368line90>
> >
> >     style: in header files, we don't put the return type on the separate line.

ok


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112040/#review37612
-----------------------------------------------------------


On Aug. 12, 2013, 7:20 p.m., Frank Meerkoetter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112040/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2013, 7:20 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> 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").
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/umscollection/UmsCollection.h 749ff81 
>   src/core-impl/collections/umscollection/UmsCollection.cpp 028966e 
> 
> Diff: http://git.reviewboard.kde.org/r/112040/diff/
> 
> 
> Testing
> -------
> 
> I have tested plugin an USB-stick containing an dmcrypt/luks encrypted ext4fs. I also tested with an USB-stick that was not encrypted.
> 
> 
> Thanks,
> 
> Frank Meerkoetter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130812/8c37859d/attachment.html>