> On April 23, 2015, 11:33 p.m., Alex Richardson wrote: > > src/lib/io/kdirwatch.cpp, line 303 > > <https://git.reviewboard.kde.org/r/123479/diff/2/?file=362725#file362725line303> > > > > Why this manual loop instead of strlen()? Does that mean that null > > characters in the middle are valid? Or, more likely, this reverse loop is > > an optimization? If so I would add a comment since it wasn't obvious to me > > straight away. > > Alex Richardson wrote: > Maybe this here is easier to read? > > if (cpath.endsWith('\0')) { > cpath.resize(cpath.lastIndexOf('\0')); > }
strlen would also iterate over the beginning which is uninteresting. and your cpath code above is wrong (should be indexOf, not lastIndexOf) and also much slower as it adds another unneccessary temporary allocation. What my loop does it skip all trailing nullbytes. I can add that as a comment if you like. - Milian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123479/#review79415 ----------------------------------------------------------- On April 23, 2015, 5:19 p.m., Milian Wolff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123479/ > ----------------------------------------------------------- > > (Updated April 23, 2015, 5:19 p.m.) > > > Review request for KDE Frameworks, David Faure, Marc Mutz, and Volker Krause. > > > Repository: kcoreaddons > > > Description > ------- > > The len in inotify_event includes nulls of the name. To prevent > them from being included in the QString/QByteArray we must filter > them manually with a recent Qt 5 dev build now. See also: > > https://codereview.qt-project.org/#/c/106473/ > > REVIEW: 123479 > > > Diffs > ----- > > autotests/kdirwatch_unittest.cpp 83b5b410e987fb45a08f8251ec65496c8074b077 > src/lib/io/kdirwatch.cpp a75fae012c59021996b46845db2e97abb5526930 > > Diff: https://git.reviewboard.kde.org/r/123479/diff/ > > > Testing > ------- > > I used the test and looked at the output and also ran it against a patched > qtbase with this: > > https://paste.kde.org/pmoue241d > > > Thanks, > > Milian Wolff > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
