Bug 14478 - [gdk] Improve FontHandling, make valgrind happy
Summary: [gdk] Improve FontHandling, make valgrind happy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 523.x (Safari 3)
Hardware: PC OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-30 09:16 PDT by Holger Freyther
Modified: 2007-07-05 06:53 PDT (History)
0 users

See Also:


Attachments
Improve Gdk Font Handling (8.10 KB, patch)
2007-06-30 09:23 PDT, Holger Freyther
no flags Details | Formatted Diff | Diff
Improve Gdk Font Handling (9.77 KB, patch)
2007-07-02 13:00 PDT, Holger Freyther
rwlbuis: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Freyther 2007-06-30 09:16:57 PDT
Valgrind helped to spot one issue with the Hash-Function and ownership of the FontPlatformData.
Comment 1 Holger Freyther 2007-06-30 09:23:31 PDT
Created attachment 15328 [details]
Improve Gdk Font Handling

-Fix various uninitialized variables
-Make the FontPlatformData::hash method at least a more reliable (I don't know if this will make a good source for the hash...)
-Move destroying of FontPlatformData resources to FontData::platformDestroy
Comment 2 Holger Freyther 2007-06-30 18:36:48 PDT
Comment on attachment 15328 [details]
Improve Gdk Font Handling

Ask for review
Comment 3 Alp Toker 2007-07-02 10:11:42 PDT
This patch helps solve some real issues. Can I suggest C++ style casts? Also

 +     * this create a deep copy

should be

 +     * this creates a deep copy
Comment 4 Holger Freyther 2007-07-02 10:15:49 PDT
(In reply to comment #3)
> This patch helps solve some real issues. Can I suggest C++ style casts? Also
> 
>  +     * this create a deep copy
> 
> should be
> 
>  +     * this creates a deep copy
> 

No this comment should be removed. This is from making FontPlatformData copyable by implementing copy c'tor and assignment operator.

I will update the patch

Comment 5 Holger Freyther 2007-07-02 13:00:38 PDT
Created attachment 15353 [details]
Improve Gdk Font Handling

-Remove the bogus comment
-Use c++ casts instead of c-casts as wished by alp.
Comment 6 Rob Buis 2007-07-03 06:16:13 PDT
Comment on attachment 15353 [details]
Improve Gdk Font Handling

>+        to FontData::platformDestroy. The ownership of these objects is hold by

Is held by.

>+        Replace c-casts with c++ casts, in this case reinterpret_cast.
>+
>+
>+
>+
>+        * platform/gdk/FontDataGdk.cpp:

I wouldn't do so many empty lines, but that is a style thing.
Code looks fine.
Comment 7 Mark Rowe (bdash) 2007-07-05 06:53:03 PDT
Landed in r24017.