D12696: Use the new uds implementation

2018-06-11 Thread Jaime Torres Amate
jtamate added a comment.


  I'm sorry, I was missing a -dev package and therefore I was not compiling kio 
sftp.
  
  A fix for this problem is in D13475 

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: lbeltrame, asturmlechner, martinkostolny, kde-frameworks-devel, bruns, 
michaelh, ngraham


D12696: Use the new uds implementation

2018-06-10 Thread Stefan Brüns
bruns added a comment.


  In D12696#276696 , @bruns wrote:
  
  > `UDS_FILE_TYPE = 13`:  32768 -> 16384 (010 -> 04)
  >  File mode changes from 'regular file' to 'directory'
  
  
  https://github.com/KDE/kio-extras/blob/master/sftp/kio_sftp.cpp#L1845
  `entry.insert(KIO::UDSEntry::UDS_FILE_TYPE, S_IFREG);`
  
  The UDS_FILE_TYPE is also set in L1873 ff to the actual file type the symlink 
points to.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: lbeltrame, asturmlechner, martinkostolny, kde-frameworks-devel, bruns, 
michaelh, ngraham


D12696: Use the new uds implementation

2018-06-10 Thread Stefan Brüns
bruns added a comment.


  In D12696#276660 , @martinkostolny 
wrote:
  
  >
  
  
  `udsField=33554445, value=16384`
  `33554445 = 0x20d = UDS_NUMBER = 0x0200 |  UDS_FILE_TYPE = 13`
  https://api.kde.org/frameworks/kio/html/classKIO_1_1UDSEntry.html
  
  `UDS_NAME = 6`: "linked-folder"
  `UDS_LINK_DEST = 14`: "/mnt/add/folders/linked-folder"
  `UDS_FILE_TYPE = 13`:  32768 -> 16384 (010 -> 04)
  File mode changes from 'regular file' to 'directory'

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: lbeltrame, asturmlechner, martinkostolny, kde-frameworks-devel, bruns, 
michaelh, ngraham


D12696: Use the new uds implementation

2018-06-10 Thread Martin Kostolný
martinkostolny added a comment.


  I've managed to get more info about the crash also by following this guide:
  
https://community.kde.org/Guidelines_and_HOWTOs/Debugging/Debugging_IOSlaves#Attaching_gdb_to_an_io-slave
  
  Here is backtrace from gdb: F5905112: udsentry_bt.txt 

  
  Here is debug log of sftp slave - I've added a debug line to each insert() 
method in `UDSEntryPrivate`: F5905116: udsentry_debug.log 

  
  Performed test was done against a remote ssh server (Solaris), therefore IP 
and paths are changed by me in the debug log. But I get the same results when 
testing against my own localhost ssh server. I can do other tests on demand and 
on localhost :).
  
  I have Arch Linux with latest updates, Qt 5.11.0 installed. The rest of KDE 
is running in kdesrc-build environment, which was updated probably 2 days ago.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: lbeltrame, asturmlechner, martinkostolny, kde-frameworks-devel, bruns, 
michaelh, ngraham


D12696: Use the new uds implementation

2018-06-09 Thread Luca Beltrame
lbeltrame added a comment.


  I wonder if this is related to timeline:/ no longer working properly in the 
file dialog. (didn't investigate yet).

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: lbeltrame, asturmlechner, martinkostolny, kde-frameworks-devel, bruns, 
michaelh, ngraham


D12696: Use the new uds implementation

2018-06-09 Thread Andreas Sturmlechner
asturmlechner added a comment.


  No crash in my case, but this commit makes links with sftp:/ inaccessible.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: asturmlechner, martinkostolny, kde-frameworks-devel, bruns, michaelh, 
ngraham


D12696: Use the new uds implementation

2018-06-07 Thread Martin Kostolný
martinkostolny added a comment.


  Thanks for guidance regarding getting additional debug info. I'll try to get 
it soon :).

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: martinkostolny, kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-06-07 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> martinkostolny wrote in udsentry.cpp:106
> sftp slave crashes here when entering directory with links

Any chance you can attach a debugger (or load the core file with gdb) and 
provide the values of 'udsField' and 'value'?

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: martinkostolny, kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-06-07 Thread Jaime Torres Amate
jtamate added a comment.


  In D12696#275105 , @martinkostolny 
wrote:
  
  > Hi! Probably after this commit sftp slave crashes when showing a directory 
with links. Please see my code comment. Can you also reproduce or is it on my 
side only?
  
  
  I'm sorry, I can't reproduce it with dolphin. (symbolic links and hard links 
and sftp://127.0.0.1).
  Could you get a backtrace with more information, at least who is calling 
udsentry.cpp, line 107? Probably you can get it running the program inside gdb 
or under valgrind. Or you should compile the program and dependencies with 
-DCMAKE_BUILD_TYPE=RelWithDebInfo

INLINE COMMENTS

> bruns wrote in kfileitem.cpp:198
> Ignore this one ..

In fact, I've tried creating a method to delete in one pass some selected 
values and then insert again, but it was 10 milliseconds slower than calling 
replace directly.

> dfaure wrote in udsentry.h:319
> heh, nice syntax ;-)

This syntax must be kept in the case of the QDebug << operator, otherwise 
clang++ doesn't compile.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: martinkostolny, kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-06-07 Thread Martin Kostolný
martinkostolny added a comment.


  Hi! Probably after this commit sftp slave crashes when showing a directory 
with links. Please see my code comment. Can you also reproduce or is it on my 
side only?
  
  Here is a crash log:
  
log_kio_sftp: readdir:  "/mnt/ext/files" , details:  "2"
ASSERT: "std::find_if(storage.cbegin(), storage.cend(), [udsField](const 
Field ) {return entry.m_index == udsField;}) == storage.cend()" in file 
/home/kotelnik/kde/src/frameworks/kio/src/core/udsentry.cpp, line 107
kioslave: ### CRASH ## protocol = kio_sftp pid = 20126 signal = 6
/home/kotelnik/kde/usr/lib64/libKF5KIOCore.so.5(+0x9d42d)[0x7fa660a1042d]
/usr/lib/libc.so.6(+0x368f0)[0x7fa6689888f0]
/usr/lib/libc.so.6(gsignal+0x10b)[0x7fa66898886b]
/usr/lib/libc.so.6(abort+0x129)[0x7fa66897340e]
/usr/lib/libQt5Core.so.5(+0x8033c)[0x7fa6696c433c]
/usr/lib/libQt5Core.so.5(_Z11qt_assert_xPKcS0_S0_i+0x0)[0x7fa6696c3768]
/home/kotelnik/kde/usr/lib64/libKF5KIOCore.so.5(+0x1033bb)[0x7fa660a763bb]

/home/kotelnik/kde/usr/lib64/libKF5KIOCore.so.5(_ZN3KIO8UDSEntry6insertEjx+0x33)[0x7fa660a78403]

/home/kotelnik/kde/usr/lib64/plugins/kf5/kio/sftp.so(+0x12371)[0x7fa657ee4371]

/home/kotelnik/kde/usr/lib64/libKF5KIOCore.so.5(_ZN3KIO9SlaveBase8dispatchEiRK10QByteArray+0x86f)[0x7fa660a12961]

/home/kotelnik/kde/usr/lib64/libKF5KIOCore.so.5(_ZN3KIO9SlaveBase12dispatchLoopEv+0x2a0)[0x7fa660a0e2aa]

/home/kotelnik/kde/usr/lib64/plugins/kf5/kio/sftp.so(kdemain+0x264)[0x7fa657ed88ca]

INLINE COMMENTS

> udsentry.cpp:106
> +Q_ASSERT(udsField & KIO::UDSEntry::UDS_NUMBER);
> +Q_ASSERT(std::find_if(storage.cbegin(), storage.cend(),
> +[udsField](const Field ) {return 
> entry.m_index == udsField;}) == storage.cend());

sftp slave crashes here when entering directory with links

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: martinkostolny, kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-10 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:e80c31163170: Use the new uds implementation (authored by 
jtamate).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33929=33957

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-10 Thread David Faure
dfaure accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-10 Thread Jaime Torres Amate
jtamate updated this revision to Diff 33929.
jtamate marked 8 inline comments as done and 3 inline comments as done.
jtamate edited the summary of this revision.
jtamate added a comment.


  Added the documentation for insert.
  Removed the () from the QDataStream& operators, but must be kept for QDebug 
or clang++ will not compile.
  Adjusted the reserved amounts.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33889=33929

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-09 Thread David Faure
dfaure added a comment.


  Starting to look good ;-)

INLINE COMMENTS

> udsentry.h:143
>  /**
>   * insert field with string value
>   * @param field numeric field id

The old insert() should be documented that it asserts if the entry already 
exists, and that one should use replace() instead. This will minimize surprises 
in case anyone hits this.

> udsentry.h:319
>  QSharedDataPointer d;
> +friend KIOCORE_EXPORT QDataStream& (::operator<<) (QDataStream , const 
> KIO::UDSEntry );
> +friend KIOCORE_EXPORT QDataStream& (::operator>>) (QDataStream , 
> KIO::UDSEntry );

heh, nice syntax ;-)

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-09 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> bruns wrote in kfileitem.cpp:198
> Wouldn't it be better to call m_entry.clear() and m_entry.insert(...) here?
> 
> For the call from the constructor, m_entry is empty, so you save the 
> std::find_if() in the replace method calls.
> 
> For any other caller, is there any useful info in m_entry() which is still 
> valid when calling init() again?

Ignore this one ..

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-09 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> kfileitem.cpp:198
>  const QByteArray pathBA = QFile::encodeName(path);
>  if (QT_LSTAT(pathBA.constData(), ) == 0) {
> +m_entry.replace(KIO::UDSEntry::UDS_DEVICE_ID,   
> buf.st_dev);

m_entry.reserve(9)

> kfileitem.cpp:198
>  const QByteArray pathBA = QFile::encodeName(path);
>  if (QT_LSTAT(pathBA.constData(), ) == 0) {
> +m_entry.replace(KIO::UDSEntry::UDS_DEVICE_ID,   
> buf.st_dev);

Wouldn't it be better to call m_entry.clear() and m_entry.insert(...) here?

For the call from the constructor, m_entry is empty, so you save the 
std::find_if() in the replace method calls.

For any other caller, is there any useful info in m_entry() which is still 
valid when calling init() again?

> udsentry.cpp:184
> +{
> +s << static_cast(storage.size());
>  

quint32, see ..::load(...) below.

> udsentry.cpp:341
>  {
>  reserve(9);
>  insert(UDS_NAME,name);

I can count 10 inserts below ...

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-09 Thread Jaime Torres Amate
jtamate updated this revision to Diff 33889.
jtamate edited the summary of this revision.
jtamate added a comment.
Restricted Application added a subscriber: kde-frameworks-devel.


  Direct friendship with the operators.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33815=33889

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-08 Thread Jaime Torres Amate
jtamate added inline comments.

INLINE COMMENTS

> dfaure wrote in udsentry.cpp:454
> Hmm why can't this be the friend function directly?
> 
> I don't like the added global functions in the public header...

> Hmm why can't this be the friend function directly?

Because the compiler (clang++ in this case) doesn't know which one to apply.
Unless both definitions are equivalent (in source and binary) and the later can 
be removed. That is something I don't known.

  src/core/slavebase.cpp:728:14: error: use of overloaded operator '<<' is 
ambiguous (with operand types 'QDataStream' and 'const KIO::UDSEntry')
  KIO_DATA << entry;
   ^  ~
  src/core/udsentry.h:315:40: note: candidate function
  friend QDataStream << (QDataStream , const KIO::UDSEntry );
  ^
  src/core/udsentry.h:361:29: note: candidate function
  KIOCORE_EXPORT QDataStream << (QDataStream , const KIO::UDSEntry 
);
  ^

> I don't like the added global functions in the public header...

Me neither, but otherwise clang++will not be able to find the function.

  src/core/udsentry.h:314:19: error: no function named 'save' with type 'void 
(QDataStream &, const KIO::UDSEntry &)' was found in the specified scope
  friend void ::save(QDataStream &, const KIO::UDSEntry &);
^

> dfaure wrote in udsentry.cpp:45
> 1ms is relative to a benchmark which isn't clear when reading this code in 
> other contexts. As someone said in another review, do we still need this 
> operator== anyway, given that you pass lambdas to find_if()?

ok, ok, bye bye ==

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-08 Thread David Faure
dfaure added inline comments.

INLINE COMMENTS

> udsentry.cpp:454
>  
> -KIOCORE_EXPORT QDebug operator<<(QDebug stream, const KIO::UDSEntry )
> +KIOCORE_EXPORT QDataStream <<(QDataStream , const UDSEntry )
>  {

Hmm why can't this be the friend function directly?

I don't like the added global functions in the public header...

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-08 Thread Jaime Torres Amate
jtamate updated this revision to Diff 33815.
jtamate added a comment.


  Make some friends functions.
  A bad type in copy didn't allowed me the use of the module functions 
save/load/debugUDSEntry,

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33809=33815

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-08 Thread Jaime Torres Amate
jtamate updated this revision to Diff 33809.
jtamate added a comment.


  Don't use java style.
  As UDSEntryPrivate is private, declare public methods save, load and 
debugUDSEntry, otherwise I couldn't access the private d pointer from the << 
and >> operators.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33758=33809

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-07 Thread David Faure
dfaure added a comment.


  Oh OK I had misunderstood your comment.
  
  Right, those methods don't have to be inline.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-07 Thread Stefan Brüns
bruns added a comment.


  In D12696#259292 , @dfaure wrote:
  
  > I requested it, for proper encapsulation
  
  
  Whats the difference between
  
class UDSEntryPrivate
{
  void load(...) {
// function body
  }
};
  
  and
  
class UDSEntryPrivate
{
  void load(...);
};

void UDSEntryPrivate::load(...)
{
// function body
}
  
  from an encapsulation perspective?

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-07 Thread David Faure
dfaure added a comment.


  I requested it, for proper encapsulation

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-07 Thread Stefan Brüns
bruns added inline comments.

INLINE COMMENTS

> udsentry.cpp:132
> +}
> +static void save(QDataStream , const UDSEntry )
> +{

Is there a reason to define the methods inside the class declaration? The diff 
would be significantly smaller if you kept the definition separate.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: bruns, michaelh, ngraham


D12696: Use the new uds implementation

2018-05-07 Thread David Faure
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> udsentry.cpp:132
> +}
> +static void save(QDataStream , const UDSEntry )
> +{

strange that it's static, "a.d" could be this, if it wasn't static.

> udsentry.cpp:151
> +}
> +static void load(QDataStream , UDSEntry )
> +{

same

> udsentry.cpp:265
>  
> -void insert(uint uds, const Field& field)
> +static void debugUDSEntry(QDebug stream, const KIO::UDSEntry )
>  {

should also not be static

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: michaelh, ngraham, bruns


D12696: Use the new uds implementation

2018-05-07 Thread Jaime Torres Amate
jtamate updated this revision to Diff 33758.
jtamate marked 9 inline comments as done.
jtamate edited the summary of this revision.
jtamate added a comment.


  Hopefully, all the issues fixed.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12696?vs=33622=33758

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: michaelh, ngraham, bruns


D12696: Use the new uds implementation

2018-05-06 Thread David Faure
dfaure added a comment.


  First line of commit log should have changelog in mind, so more context 
needed. Maybe rather something like "KIO::UDSEntry: switch to a single 
std::vector for more performance".

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: michaelh, ngraham, bruns


D12696: Use the new uds implementation

2018-05-06 Thread David Faure
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Looks good.

INLINE COMMENTS

> udsentry.cpp:45
> +inline Field(const uint index, long long value = 0) : m_long(value), 
> m_index(index) {}
> +// This operator helps to gain 1ms just comparing the key
> +inline bool operator == (const Field ) const {

1ms is relative to a benchmark which isn't clear when reading this code in 
other contexts. As someone said in another review, do we still need this 
operator== anyway, given that you pass lambdas to find_if()?

> udsentry.cpp:55
> +std::vector storage;
> +public:
> +void reserve(int size)

remove this "public:", we're already in the public section.

But actually, now that there are methods wrapping all usage of "storage", how 
about making "storage" private? (moving it to the end of the class, to respect 
the standard order in Qt/KF5 code: public / protected / private).

Actually, the Field class definition can move with it too.

Ah, I see a few remaining uses from the outside, how about moving it all in for 
proper encapsulation? Some save/load methods with QDataStream, and a method 
that takes QDebug...

> udsentry.cpp:118
> +}
> +QList listFields() const
> +{

#ifndef KIOCORE_NO_DEPRECATED

like the only caller

> udsentry.cpp:120
> +{
> +QList res;
> +for (auto it = storage.cbegin(), end = storage.cend(); it != end; 
> ++it) {

res.reserve(storage.count());

> udsentry.cpp:128
> +{
> +QVector res;
> +for (auto it = storage.cbegin(), end = storage.cend(); it != end; 
> ++it) {

res.reserve(storage.count());

> udsentry.cpp:414
> +default:
> +return QString("Unknow uds field %1").arg(field);
> +}

Typo: unknown

> udsentry.h:320
> + */
> +void replaceOrInsert(uint field, const QString );
> +

@since 5.47

> udsentry.h:327
> + */
> +void replaceOrInsert(uint field, long long l);
> +

@since 5.47

Should these be called just replace() like QMultiMap::replace also means 
"replace or insert" ?

> udsentry.h:333
> + */
> +static QString nameOfUdsField(uint field);
>  };

I see you're using this internally, is it needed in the public API? I'd just 
make it internal.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: michaelh, ngraham, bruns


D12696: Use the new uds implementation

2018-05-04 Thread Jaime Torres Amate
jtamate created this revision.
jtamate added reviewers: dfaure, Frameworks.
Restricted Application added a project: Frameworks.
jtamate requested review of this revision.

REVISION SUMMARY
  This new implementation is the fastest in the autotests.
  The replaceOrInsert in KFileItem::setName is due to renaming a file.
  The replaceOrInsert in ListJobPrivate::slotListEntries is due to Kfind.

TEST PLAN
  It should be used extensively at least one release cycle before pushing it.
  
  Passes the autotests.
  Don't assert in any kde program that uses kio.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/core/kfileitem.cpp
  src/core/listjob.cpp
  src/core/udsentry.cpp
  src/core/udsentry.h

To: jtamate, dfaure, #frameworks
Cc: michaelh, bruns