Blob.cpp fails to build with some gcc versions because of signed/unsigned comparison

[Praveen Jadhav]

WebKit build fails for EFL port with below error

/home/praveen.j/WebKit/Source/WebCore/fileapi/Blob.cpp: In member function ‘long long unsigned int WebCore::Blob::size() const’:
/home/praveen.j/WebKit/Source/WebCore/fileapi/Blob.cpp:127:69: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
cc1plus: all warnings being treated as errors
make[2]: *** [Source/WebCore/CMakeFiles/WebCore.dir/fileapi/Blob.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Source/WebCore/CMakeFiles/WebCore.dir/all] Error 2
make: *** [all] Error 2

[Praveen Jadhav]

Created attachment 231056 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 231056 [details]
Patch

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

Sorry for the breakage - efl-wk2 bot was green, and efl one does not work.

> Source/WebCore/fileapi/Blob.cpp:127
> -        m_size = (actualSize <= std::numeric_limits<long long>::max()) ? static_cast<long long>(actualSize) : 0;
> +        m_size = (actualSize <= std::numeric_limits<unsigned long long>::max()) ? static_cast<long long>(actualSize) : 0;

The purpose of this check is to verify that the number fits into long long - we already know that it fits into an unsigned long long!

if it complains about signed/unsigned comparison, you probably need something like

actualSize <= static_cast<unsigned long long>(std::numeric_limits<long long>::max()))

[Praveen Jadhav]

Well, its strange that no one else got this error(efl buildbot is successful). I am not sure if this is because of some mismatch in my PC. Will try to resolve this locally.

Thanks!

[Alexey Proskuryakov]

*** Bug 132947 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

As mentioned in duplicate, perhaps we have an existing less wordy solution for this? Perhaps something in CheckedArithmetic.h?

[Alexey Proskuryakov]

Seems like isInBounds<long long>(actualSize) should do the trick.

[Tanay]

Created attachment 231560 [details]
Patch

[Tanay]

(In reply to comment #6)
> Seems like isInBounds<long long>(actualSize) should do the trick.

Yes it fixes the build issue. Uploaded a patch with changes.

[Alexey Proskuryakov]

Comment on attachment 231560 [details]
Patch

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

> Source/WebCore/fileapi/Blob.cpp:127
> +        m_size = (WTF::isInBounds<long long>(actualSize)) ? static_cast<long long>(actualSize) : 0;

Does it compile without "WTF::"? Please remove the prefix when landing if it does.

[Tanay]

(In reply to comment #9)
> (From update of attachment 231560 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231560&action=review
> 
> > Source/WebCore/fileapi/Blob.cpp:127
> > +        m_size = (WTF::isInBounds<long long>(actualSize)) ? static_cast<long long>(actualSize) : 0;
> 
> Does it compile without "WTF::"? Please remove the prefix when landing if it does.

Without 'WTF::' compiler is not able to resolve the function. We should retain it.

[WebKit Commit Bot]

Comment on attachment 231560 [details]
Patch

Clearing flags on attachment: 231560

Committed r169329: <http://trac.webkit.org/changeset/169329>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.