378700 - New fields in PangoAnalyis of pango 1.16 need initializing in CreateGlyphRunsFast

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Karl Tomlinson (:karlt)]

Attachment: initialization and memory leak plug

In pango 1.16.0 new fields gravity and flags have been added to the
PangoAnalysis struct.  These need to be initialized in
gfxPangoFontGroup::CreateGlyphRunsFast().

(The new fields have been added within the padding present in pango 1.14:
http://bugzilla.gnome.org/show_bug.cgi?id=351496)

The attached patch initializes the new fields and also removes a memory leak
in the case where AddGlyphRun fails.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 262724 [details] [diff] [review]
initialization and memory leak plug

Change the struct initializer to memset.

Comment 2

[Robert Sayre]

oh good, I was seeing this in valgrind today

Comment 3

[Karl Tomlinson (:karlt)]

The struct initilizer came from code in pango-1.16.2/pango/break.c:

  PangoAnalysis analysis = { 0 };

Comment 4

[Karl Tomlinson (:karlt)]

For C++:

X3J16/96–0225 WG21/N1043

8.5.1 Aggregates

7 If there are fewer initializers in the list than there are members in the
  aggregate, then each member not explicitly initialized shall be initialized
  with a value of the form T() (5.2.3), where T represents the type of the
  uninitialized member.
  [Example:

              struct S { int a; char* b; int c; };
              S ss = { 1, "asdf" };

  initializes ss.a with 1, ss.b with "asdf", and ss.c with the value of an
  expression of the form int(), that is, 0. ]

5.2.3 Explicit type conversion (functional notation)

2 The expression T(), where T is a simple-type-specifier (7.1.5.2) for a
  non-array complete object type or the (possibly cv-qualified) void type,
  creates an rvalue of the specified type, whose value is determined by
  default-initialization (8.5). [Note: if T is a non-class type that is
  cv-qualified, the cv-qualifiers are ignored when determining the type of the
  resulting rvalue (3.10). ]

8.5 Initilizers

5
  To default-initialize an object of type T means:
  — if T is a non-POD class type (9), the default constructor for T is called
    (and the initialization is ill-formed if T has no accessible default
    constructor);
  — if T is an array type, each element is default-initialized;
  — otherwise, the storage for the object is zero-initialized.


So { 0 } should do what we want.

Comment 5

[Karl Tomlinson (:karlt)]

However, XL C/C++ V8.0 seems to be reading a different standard!

http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.xlcpp8a.doc/language/ref/strin.htm:

 "the initial value of uninitialized structure members depends on the storage
  class associated with the structure or union variable. In a structure declared
  as static, any members that are not initialized are implicitly initialized to
  zero of the appropriate type; the members of a structure with automatic
  storage have no default initialization."

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: patch using memset

If XL C++ really does what it says and we want to support it or other non-compliant compilers then this patch may be needed.

Comment 7

[Karl Tomlinson (:karlt)]

CreateGlyphRunsFast() will be removed for bug 357637 so the patches here won't be needed unless there is a problem with attachment 263898 [details] [diff] [review] in bug 357637.

Comment 8

[Karl Tomlinson (:karlt)]

Fixed by removal of CreateGlyphRunsFast() in bug 357637.