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

Fabian Vogt fabian at ritter-vogt.de
Fri May 20 06:51:23 UTC 2016



> On Mai 19, 2016, 11:17 nachm., Michael Pyne wrote:
> > 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!

I didn't know this aligned implementation actually had an upstream, it's actually fixed there, the latest source is on GitHub: https://github.com/aappleby/smhasher/blob/master/src/MurmurHash1.cpp#L87


- Fabian


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


On Mai 19, 2016, 6:13 nachm., Fabian Vogt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127968/
> -----------------------------------------------------------
> 
> (Updated Mai 19, 2016, 6:13 nachm.)
> 
> 
> 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/20160520/46f50fd0/attachment.html>


More information about the Kde-frameworks-devel mailing list