Review Request 127968: kshareddatacache: Fix invalid use of & to avoid unaligned reads

Michael Pyne mpyne at kde.org
Thu May 19 23:17:38 UTC 2016


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127968/#review95630
-----------------------------------------------------------


Ship it!




This is an error upstream in the MurmurHash 1.0 aligned implementation (which still contains the error, see https://sites.google.com/site/murmurhash/MurmurHashAligned.cpp?attredirects=0 )

In our case KSharedDataCache will never pass an unaligned block into the hash function so 'align' was always 0 (and boolean false) anyways. So I concur with not bumping the cache version and agree with your patch. Thanks for taking a look!

- Michael Pyne


On May 19, 2016, 6:13 p.m., Fabian Vogt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127968/
> -----------------------------------------------------------
> 
> (Updated May 19, 2016, 6:13 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> That the & is wrong here is obvious:
> align can never be 2 inside the if condition as ```(2 & bool)``` is always false,
> yet the switch on align contains a case for 2.
> Instead it drops into the while loop for an 4-byte aligned pointer.
> 
> This doesn't change the returned values on platforms where unaligned reads
> are permitted, so I didn't bump the version.
> 
> 
> Diffs
> -----
> 
>   src/lib/caching/kshareddatacache.cpp 50bbf64 
> 
> Diff: https://git.reviewboard.kde.org/r/127968/diff/
> 
> 
> Testing
> -------
> 
> Ran make test, all passed.
> 
> 
> Thanks,
> 
> Fabian Vogt
> 
>

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


More information about the Kde-frameworks-devel mailing list