[webkit-reviews] review granted: [Bug 125674] [ARM] Fix unsafe memory load/store from the argument encoder/decoder : [Attachment 219147] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 125674: [ARM] Fix unsafe memory load/store from the argument
encoder/decoder
https://bugs.webkit.org/show_bug.cgi?id=125674

Attachment 219147: Patch
https://bugs.webkit.org/attachment.cgi?id=219147&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=219147&action=review


> Source/WebKit2/Platform/CoreIPC/ArgumentDecoder.cpp:130
> +static void decodeValueFromBuffer(Type& value, uint8_t*& buffer)

I don’t think that a pointer that we advance should be called “buffer”, because
I think the word buffer refers to the entire buffer, not to a pointer within
it. That’s why I think Anders was using the name “buffer position”. I would
suggest naming the argument either bufferPosition or position.

> Source/WebKit2/Platform/CoreIPC/ArgumentEncoder.cpp:135
>      uint8_t* buffer = grow(sizeof(n), sizeof(n));
> -    
> -    *reinterpret_cast<bool*>(buffer) = n;
> +    copyValueToBuffer(n, buffer);

I would write these without the local variable:

    copyValueToBuffer(n, grow(sizeof(n), sizeof(n));