Bug 11822

Summary: DeprecatedStringData allocation size is 52 bytes due to struct packing
Product: WebKit Reporter: Darin Fisher (:fishd, Google) <fishd>
Component: PlatformAssignee: Darin Fisher (:fishd, Google) <fishd>
Severity: Minor    
Priority: P3    
Version: 420+   
Hardware: PC   
OS: Windows XP   
Description Flags
patch ggaren: review-

Description Darin Fisher (:fishd, Google) 2006-12-13 15:38:27 PST
DeprecatedStringData allocation size is 52 bytes due to struct packing

The comment in DeprecatedString.h says:

  // Keep this struct to <= 46 bytes, that's what the system will allocate.
  // Will be rounded up to a multiple of 4, so we're stuck at 44.

I have no idea how significant that comment is or whether it matters that DeprecatedStringData > 46 bytes on Windows.  Perhaps it would be good to wrap the declaration of that class with #pragma pack(push, 1) ... #pragma pack(pop) just in case.
Comment 1 Darin Fisher (:fishd, Google) 2006-12-15 18:43:14 PST
Created attachment 11874 [details]

I haven't determined if this actually is necessary, but here's a patch.
Comment 2 Geoffrey Garen 2006-12-23 16:24:36 PST
Comment on attachment 11874 [details]

I don't think we want to make this change without knowing what it accomplishes or being able to test it.

Building mysterious code on top of mysterious comments seems like a recipe for either (a) disaster or (b) a sequel to the Da Vinci Code.
Comment 3 Maciej Stachowiak 2006-12-23 23:07:38 PST
I think this comment was specific to the size classes of the Mac OS X system malloc. But WebKit will no longer use that, even on Mac OS X, so I think the best thing would be to remove the comment.
Comment 4 Darin Fisher (:fishd, Google) 2007-01-02 10:04:19 PST
My hope was that one of you guys would know what this comment was all about and be able to decide whether you want the same for windows or if the comment is just bogus.  Sounds like the latter, so I'll rev the patch accordingly.