Bug 227606 – Use more Span

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Use more Span

[Alex Christensen]

Created attachment 432762 [details]
Patch

[Alex Christensen]

Created attachment 432763 [details]
Patch

[Alex Christensen]

Created attachment 432835 [details]
Patch

[Alex Christensen]

Created attachment 432842 [details]
Patch

[Alex Christensen]

Created attachment 432940 [details]
Patch

[Darin Adler]

Comment on attachment 432940 [details]
Patch

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

> Source/WTF/wtf/persistence/PersistentDecoder.h:99
> +    Span<const uint8_t>::iterator m_iterator;

We should just write const uint8_t* for this type.

I don’t think this is an "iterator", it’s just a pointer. If you asked "what’s the iterator type" then, yes, you get the pointer type, but we should not try to be abstract.

I think we should keep the names m_buffer and m_bufferPosition, rather than renaming to m_span and m_iterator.

[Alex Christensen]

Created attachment 433010 [details]
Patch

[Darin Adler]

Comment on attachment 433010 [details]
Patch

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

> Source/WTF/wtf/persistence/PersistentDecoder.cpp:73
> +    // FIXME: check for underflow.
> +    if (m_bufferPosition - size >= m_buffer.begin()) {

Since we know that m_bufferPosition is >= m_buffer.begin(), we can sidestep the possibility of overflow without adding an additional check by writing this:

    if (size <= m_bufferPosition - m_buffer.begin()) {

> Source/WebKit/NetworkProcess/cache/NetworkCacheData.h:77
>      const uint8_t* data() const;
>      size_t size() const { return m_size; }

Maybe drop these eventually?

[Alex Christensen]

Committed to r279645

(In reply to Darin Adler from comment #8)
> Comment on attachment 433010 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433010&action=review
> 
> > Source/WTF/wtf/persistence/PersistentDecoder.cpp:73
> > +    // FIXME: check for underflow.
> > +    if (m_bufferPosition - size >= m_buffer.begin()) {
> 
> Since we know that m_bufferPosition is >= m_buffer.begin(), we can sidestep
> the possibility of overflow without adding an additional check by writing
> this:
> 
>     if (size <= m_bufferPosition - m_buffer.begin()) {
Great!  It also needs a static_cast<size_t> because the right side is now signed.
> 
> > Source/WebKit/NetworkProcess/cache/NetworkCacheData.h:77
> >      const uint8_t* data() const;
> >      size_t size() const { return m_size; }
> 
> Maybe drop these eventually?
Hopefully!