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