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

[Matěj Laitl]

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

Ship it!


Looks good, but please bear with those of us that are kinda stuck to code style consistency. I'd apply after releasing 2.8 (this is a little bug, but still there's a very small chance of regressions)


src/core-impl/collections/umscollection/UmsCollection.h
<http://git.reviewboard.kde.org/r/112040/#comment27784>

    style: in header files, we don't put the return type on the separate line.



src/core-impl/collections/umscollection/UmsCollection.cpp
<http://git.reviewboard.kde.org/r/112040/#comment27785>

    style: spaces around arg (in declaration it is right)



src/core-impl/collections/umscollection/UmsCollection.cpp
<http://git.reviewboard.kde.org/r/112040/#comment27786>

    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.



src/core-impl/collections/umscollection/UmsCollection.cpp
<http://git.reviewboard.kde.org/r/112040/#comment27787>

    code style nitpick: spaces around args


- Matěj Laitl


On Aug. 12, 2013, 9: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, 9: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/018e78f8/attachment-0001.html>