D15277: [RFC] kio_mtp: Move MTP device handling from kioslave to kiod-module

[David Faure]

dfaure accepted this revision.
dfaure added a comment.


  Great stuff. Only found some typos and very minor things, feel free to fix and push.

INLINE COMMENTS

> mtpdevice.cpp:119
> +{
> +    QList<QDBusObjectPath> list;
> +    for (const MTPStorage *storage : m_storages) {

Technically this is missing a

  list.reserve(m_storages.count())

but I guess these lists are pretty small in practice...

> mtpstorage.cpp:31
> + */
> +uint16_t onDataPut(void *, void *priv, uint32_t sendlen, unsigned char *data, uint32_t *putlen)
> +{

Prepend "static" to all these class-level functions (so they don't get exported, and so that the compiler knows they are only usable in this file, which can lead to more optimizations like inlining).

> kmtpdeviceinterface.cpp:35
> +    const auto storageNames = m_dbusInterface->listStorages().value();
> +    for (const QDBusObjectPath &storageName : storageNames) {
> +        m_storages.append(new KMTPStorageInterface(storageName.path(), this));

m_storages.reserve(storageNames.count())

> kmtpstorageinterface.cpp:33
> +                                                  this);
> +    m_dbusInterface->setTimeout(5 * 60 * 1000); // TODO: listening folders with a huge amount of files may take a while
> +

s/listening/listing/ ?

> org.kde.kmtp.daemon.xml:40
> +
> +        <!-- listDevices: The currently discovered and connected devices in the deamon.
> +            @devices: A list of the currently accessible devices represented as an array of D-Bus object paths.

s/deamon/daemon/

REPOSITORY
  R320 KIO Extras

REVISION DETAIL
  https://phabricator.kde.org/D15277

To: akrutzler, elvisangelaccio, ltoscano, hetzenecker, dfaure, mlaurent
Cc: mlaurent, kde-frameworks-devel, kfm-devel, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20181004/9d350f14/attachment.htm>