Bug 165630 - Add 64-bit signed LEB decode method
Summary: Add 64-bit signed LEB decode method
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-08 15:24 PST by Keith Miller
Modified: 2016-12-08 17:34 PST (History)
5 users (show)

See Also:


Attachments
Patch (19.67 KB, patch)
2016-12-08 15:26 PST, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (20.02 KB, patch)
2016-12-08 16:45 PST, Keith Miller
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2016-12-08 15:24:01 PST
Add 64-bit signed LEB decode method
Comment 1 Keith Miller 2016-12-08 15:26:22 PST
Created attachment 296576 [details]
Patch
Comment 2 Keith Miller 2016-12-08 16:45:45 PST
Created attachment 296591 [details]
Patch
Comment 3 Ryosuke Niwa 2016-12-08 16:51:33 PST
Comment on attachment 296576 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=296576&action=review

> Source/WTF/wtf/LEBDecoder.h:59
> +template<size_t maxByteLength, typename T>
> +inline bool WARN_UNUSED_RETURN decodeInt(const uint8_t* bytes, size_t length, size_t& offset, T& result)

I think it would have been better maxByteLength was obtained via traits on T.
e.g.
template <typename T> struct maxLEBBytes;
template <> maxLEBBytes<uint32_t> struct { static size_t maxByteLength = 5; }
template <> maxLEBBytes<unit64_t> struct { static size_t maxByteLength = 10; }
This would be less error prone than passing in matching template arguments.

> Source/WTF/wtf/LEBDecoder.h:65
> -    size_t last = std::min(max32BitLEBByteLength, length - offset - 1);
> +    size_t last = std::min(maxByteLength, length - offset) - 1;

You should mention -1 moving out of std::min in the change log.
Comment 4 Ryosuke Niwa 2016-12-08 16:52:02 PST
Comment on attachment 296591 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=296591&action=review

> Source/WTF/wtf/LEBDecoder.h:41
> +    const size_t maxByteLength = (numBits - 1) / 7 + 1; // numBits / 7 rounding up.

Nice!
Comment 5 Keith Miller 2016-12-08 17:34:35 PST
Committed r209586: <http://trac.webkit.org/changeset/209586>