[Webkit-unassigned] [Bug 42759] Always freeing m_logAttrs in TextBreakIteratorGtk.cpp: setUpIterator()

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

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




--- Comment #2 from Martin Robinson <mrobinson at webkit.org>  2011-05-03 10:38:35 PST ---
(From update of attachment 92060)
View in context: https://bugs.webkit.org/attachment.cgi?id=92060&action=review

Thanks for your contribution! What's your motivation for this change? Does it provide a measurable performance improvement in breaking text? 

This patch is missing a ChangeLog entry, which is a requirement for all WebKit patches. (see http://www.webkit.org/coding/contributing.html). I recommend using the ./Tools/Scripts/webkit-patch tool to submit your patches. It will warn you about style violations before you even submit your patch. You can also use check-webkit-style. Here are the coding style guidelines: http://www.webkit.org/coding/coding-style.html

> Source/WebCore/platform/text/gtk/TextBreakIteratorGtk.cpp:194
> +    int m_logAttrsLen;

Should be m_logAttrsLength.

> Source/WebCore/platform/text/gtk/TextBreakIteratorGtk.cpp:200
> +    , m_logAttrs(NULL)

In WebKit we use 0 instead of NULL except in situations where it would cause an error or warning.

> Source/WebCore/platform/text/gtk/TextBreakIteratorGtk.cpp:231
> +    } else {
> +        memset(static_cast<void*>(iterator->m_logAttrs), 0, charLength + 1);
> +    }

Since you have only one statement in this block, the style guide says you must omit the curly braces.

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