Bug 225593 – Remove unnecessary comparison and casting in GlyphBuffer.h

• Status: UNCONFIRMED
• Component: New Bugs

[Eleni Maria Stea]

Variable result in checkedStringOffsetAt in GlyphBuffer.h is unsigned. Therefore, it can never be < 0 and static cast to unsigned before comparison is unnecessary.

[Eleni Maria Stea]

Created attachment 428166 [details]
Patch

[Eleni Maria Stea]

I had only built this on Linux/GCC and received warnings about unsigned which were fixed when I removed the cast. But it seems that it was required (EWS failure) above.

This bug report is invalid.

[Sam Weinig]

Comment on attachment 428166 [details]
Patch

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

> Source/WebCore/platform/graphics/GlyphBuffer.h:81
> -        if (result < 0 || static_cast<unsigned>(result) >= stringLength)
> +        if (result >= stringLength)

This is not true on all platforms. When USE(CG) is true, GlyphBufferStringOffset (the type of result) is CFIndex which is 'signed long'.

The real issue that we should try to solve is using the same types here if we can.

[Sam Weinig]

Re-opening, I think we should at least see if we can converge on the same type.

[Sam Weinig]

Myles do you know

[Sam Weinig]

Myles, can you think of anything good we can do to make it so non-CG ports and CG ports can use these types consistently?

[Eleni Maria Stea]

I agree, sorry I closed it. Besides I think that at least the part where we check it's < 0 should be removed because we always use unsigned: either unsigned int or unsigned long

Comment on attachment 428166 [details]
Patch

Remove bug

Comment on attachment 428166 [details]
Patch

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

> Source/WebCore/platform/graphics/GlyphBuffer.h:79
>      {

Revert all properties

[Sam Weinig]

Well, as I said above, USE(CG) uses CFIndex which is signed (either signed long or signed long long).

[Radar WebKit Bug Importer]

<rdar://problem/90248625>