Review Request 123479: Adapt to Qt 5.6 changes and prevent nullbytes in QStrings.

Milian Wolff mail at milianw.de
Fri Apr 24 09:10:45 UTC 2015



> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150424/d1585483/attachment.html>


More information about the Kde-frameworks-devel mailing list