[Webkit-unassigned] [Bug 46695] [Qt] Invalid pointer access & incomplete memcmp in setUpIterator

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=46695


Andreas Kling <kling at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #82806|review?                     |review-
               Flag|                            |




--- Comment #22 from Andreas Kling <kling at webkit.org>  2011-02-18 02:47:23 PST ---
(From update of attachment 82806)
View in context: https://bugs.webkit.org/attachment.cgi?id=82806&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

This line should be replaced, either mention what tests are added, or explain why none are needed.

> Source/WebCore/ChangeLog:9
> +

I'm missing a comment block here explaining that you're adding a way for TextBreakIterators to hold a reference to the last string they operate on for reuse purposes.

> Source/WebCore/platform/text/qt/TextBreakIteratorQt.cpp:48
> +        TextBreakIterator(QTextBoundaryFinder::BoundaryType type, const UChar* string, int length, PassRefPtr<StringImpl> origText = 0)

"origText" is a bad variable name, we stay away from abbreviations in WebKit style. Repeated below.

> Source/WebCore/platform/text/gtk/TextBreakIteratorGtk.cpp:244
> +TextBreakIterator* lineBreakIterator(const UChar* string, int length, PassRefPtr<StringImpl> origText = 0 /* used for memory-safe recycling of Iterator */)

No need for the " = 0 /* used for memory-safe recycling of Iterator */" here.

> Source/WebCore/rendering/break_lines.h:28
> +typedef PassRefPtr<StringImpl> Blerk; 

Blerk?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.