Bug 88344 – Fix FastMalloc.cpp compile error for MSVC in 64-bit

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Alex Christensen]

Source/WTF/wtf/FastMalloc.cpp does not compile for 64 bit machines.  Line 2477 tries to make an array of size 0.  I suggest this line be changed from 

  char pad_[(64 - (sizeof(TCMalloc_Central_FreeList) % 64)) % 64];

(which evaluates to 0 when sizeof(TCMalloc_Central_FreeList) is a multiple of 64) to this:

  char pad_[64 - (sizeof(TCMalloc_Central_FreeList) % 64)];

This will not change anything for a 32 bit machine, and it will create a 64 byte pad on a 64 bit machine, which will use a little extra memory, but it will compile correctly and still pad to a multiple of 64 bytes.

[Alex Christensen]

Created attachment 145851 [details]
patch

patch

[Alexey Proskuryakov]

Thank you for preparing a nice patch with ChangeLog! To follow through on the process, please set "review?" flag via patch details, so that it gets into review queue.

I have a question: what platforms does this actually fix? WebKit certainly compiles fine for 64-bit Mac, so the patch description is incomplete.

[Alex Christensen]

Comment on attachment 145851 [details]
patch

Visual Studio x64 compiler sees an array of length 0 without this patch.

[Ryosuke Niwa]

Comment on attachment 145851 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145851&action=review

> Source/WTF/wtf/FastMalloc.cpp:2477
> -  char pad_[(64 - (sizeof(TCMalloc_Central_FreeList) % 64)) % 64];
> +    char pad[64 - (sizeof(TCMalloc_Central_FreeList) % 64)];

Clearly, whoever wrote this code didn't test it :(

[Ryosuke Niwa]

Comment on attachment 145851 [details]
patch

Assuming you're not a committer, if you wanted your patch to be committed, please let me know if set cq? flag as well so that other committers may land this patch on behalf of you.

[Ryosuke Niwa]

(In reply to comment #5)
> (From update of attachment 145851 [details])
> Assuming you're not a committer, if you wanted your patch to be committed, please let me know if set cq? flag as well so that other committers may land this patch on behalf of you.

Ugh... I meant to say:

Please let me know if you wanted your patch to be committed, or set cq? flag as well so that other committers may land this patch on behalf of you.

[Ryosuke Niwa]

Comment on attachment 145851 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=145851&action=review

>> Source/WTF/wtf/FastMalloc.cpp:2477
>>   private:
>> -  char pad_[(64 - (sizeof(TCMalloc_Central_FreeList) % 64)) % 64];
>> +    char pad[64 - (sizeof(TCMalloc_Central_FreeList) % 64)];
> 
> Clearly, whoever wrote this code didn't test it :(

On my second thought you should wrap this with
#if sizeof(TCMalloc_Central_FreeList) % 64
#endif
so that we don't increase the size when sizeof(TCMalloc_Central_FreeList) is a multiple of 64.

[Alex Christensen]

The preprocessor cannot use the sizeof operator.  Since this would add a maximum of 64 bytes per TCMalloc_Central_FreeListPadded and there is only an array of kNumClasses of these objects, and kNumClasses is 68, this would use a maximum of 4420 bytes.  I don't think this is anything to worry about.

Commit it!

(In reply to comment #7)
> (From update of attachment 145851 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145851&action=review
> 
> >> Source/WTF/wtf/FastMalloc.cpp:2477
> >>   private:
> >> -  char pad_[(64 - (sizeof(TCMalloc_Central_FreeList) % 64)) % 64];
> >> +    char pad[64 - (sizeof(TCMalloc_Central_FreeList) % 64)];
> > 
> > Clearly, whoever wrote this code didn't test it :(
> 
> On my second thought you should wrap this with
> #if sizeof(TCMalloc_Central_FreeList) % 64
> #endif
> so that we don't increase the size when sizeof(TCMalloc_Central_FreeList) is a multiple of 64.

[Alexey Proskuryakov]

> The preprocessor cannot use the sizeof operator.

Why?

[Ryosuke Niwa]

(In reply to comment #8)
> The preprocessor cannot use the sizeof operator.  Since this would add a maximum of 64 bytes per TCMalloc_Central_FreeListPadded and there is only an array of kNumClasses of these objects, and kNumClasses is 68, this would use a maximum of 4420 bytes.  I don't think this is anything to worry about.

Oops, you're right. However, we can use template specialization based on sizeof.

[Alexey Proskuryakov]

Sorry, yes, why Ryosuke said.

[Alex Christensen]

Using template specialization would change a lot of the pointer types, especially when OS(DARWIN) is true.  This would require a bit more surgery on this cpp file.  Would it be worth restructuring the whole file just to save 6k of memory in a global variable with one instance?

(In reply to comment #10)
> (In reply to comment #8)
> > The preprocessor cannot use the sizeof operator.  Since this would add a maximum of 64 bytes per TCMalloc_Central_FreeListPadded and there is only an array of kNumClasses of these objects, and kNumClasses is 68, this would use a maximum of 4420 bytes.  I don't think this is anything to worry about.
> 
> Oops, you're right. However, we can use template specialization based on sizeof.

[Alex Christensen]

scratch that.  I got it working with specialized templates and am uploading a new patch.

(In reply to comment #12)
> Using template specialization would change a lot of the pointer types, especially when OS(DARWIN) is true.  This would require a bit more surgery on this cpp file.  Would it be worth restructuring the whole file just to save 6k of memory in a global variable with one instance?
> 
> (In reply to comment #10)
> > (In reply to comment #8)
> > > The preprocessor cannot use the sizeof operator.  Since this would add a maximum of 64 bytes per TCMalloc_Central_FreeListPadded and there is only an array of kNumClasses of these objects, and kNumClasses is 68, this would use a maximum of 4420 bytes.  I don't think this is anything to worry about.
> > 
> > Oops, you're right. However, we can use template specialization based on sizeof.

[Alex Christensen]

Created attachment 147167 [details]
Patch

[Build Bot]

Comment on attachment 147167 [details]
Patch

Attachment 147167 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12817031

[Ryosuke Niwa]

Comment on attachment 147167 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147167&action=review

> Source/WTF/wtf/FastMalloc.cpp:2489
> +static TCMalloc_Central_FreeListPadded<(64 - (sizeof(TCMalloc_Central_FreeList) % 64)) % 64> central_cache[kNumClasses];

Oh, you should probably put this back in struct and just pass sizeof as the argument.
Aldo, please typedef it so that users of this type doesn't need to know the trick we're using.

[Alex Christensen]

> Oh, you should probably put this back in struct and just pass sizeof as the argument.
I would have to pass at least sizeof(TCMalloc_Central_FreeList)%64.
> Aldo, please typedef it so that users of this type doesn't need to know the trick we're using.
Using typedef causes problems because of line 499.  It must already be declared as a class, or we'll have to do some major reordering.

Since this problem only arises with Visual Studio x64 compiler, I'm uploading a patch that just uses precompiler directives.

[Alex Christensen]

Created attachment 147609 [details]
Patch

[Ryosuke Niwa]

Why is it okay not to pad?

[Alex Christensen]

(In reply to comment #19)
> Why is it okay not to pad?
You're right.  Just because the size is a multiple of 64 bytes right now doesn't mean it won't change.  I'll add a padding template for MSVC.

[Alex Christensen]

Created attachment 147611 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 147611 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=147611&action=review

> Source/WTF/wtf/FastMalloc.cpp:2484
> +#if COMPILER(MSVC)
> +template <int SizeToPad> class TCMalloc_Central_FreeListPadded : public TCMalloc_Central_FreeList {
> + private:
> +    char pad[64 - SizeToPad];
> +};
> +template <> class TCMalloc_Central_FreeListPadded<0> : public TCMalloc_Central_FreeList { };
> +#else
>  class TCMalloc_Central_FreeListPadded : public TCMalloc_Central_FreeList {
>   private:
>    char pad_[(64 - (sizeof(TCMalloc_Central_FreeList) % 64)) % 64];
>  };
> +#endif

Why do we not want to use this template for other compilers? I don't see any benefit in that.
All I've asked you to do is to do
typedef TCMalloc_Central_FreeListPadded<sizeof(TCMalloc_Central_FreeList) % 64> TCMalloc_Central_FreeListPadded;

[Alex Christensen]

> Why do we not want to use this template for other compilers? I don't see any benefit in that.
The problem only exists for MSVC.  It can apparently be compiled for 64 bit and 32 bit on a mac.
> All I've asked you to do is to do
> typedef TCMalloc_Central_FreeListPadded<sizeof(TCMalloc_Central_FreeList) % 64> TCMalloc_Central_FreeListPadded;
That redefines WTF::TCMalloc_Central_FreeListPadded.

[Ryosuke Niwa]

(In reply to comment #23)
> That redefines WTF::TCMalloc_Central_FreeListPadded.

But but you can use a slightly different name for the templated version.

[Alex Christensen]

(In reply to comment #24)
> (In reply to comment #23)
> > That redefines WTF::TCMalloc_Central_FreeListPadded.
> 
> But but you can use a slightly different name for the templated version.

Doing this requires some reordering of the code, but it can be done.  I'm uploading a patch that adds the specialized template and reorders class definitions so everything compiles correctly with the new typedef.

[Alex Christensen]

Created attachment 147635 [details]
Patch

[WebKit Review Bot]

Attachment 147635 [details] did not pass style-queue:

Source/WTF/wtf/FastMalloc.cpp:1117:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1119:  The parameter name "N" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1122:  The parameter name "N" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1125:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1131:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1131:  tc_length is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1159:  Use 0 or null instead of NULL (even in *comments*).  [readability/null] [4]
Source/WTF/wtf/FastMalloc.cpp:1163:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1164:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1165:  Use 0 or null instead of NULL (even in *comments*).  [readability/null] [4]
Source/WTF/wtf/FastMalloc.cpp:1176:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1198:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1199:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1203:  locked_size_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1207:  lock_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1210:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1210:  Extra space between size_t and size_class_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1210:  size_class_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1211:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1211:  Extra space between Span and empty_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1211:  empty_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1212:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1212:  Extra space between Span and nonempty_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1212:  nonempty_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1213:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1213:  Extra space between size_t and counter_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1213:  counter_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1215:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1218:  tc_slots_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1220:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1222:  used_slots_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1223:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1226:  cache_size_Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Fa..." exit_code: 1
 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1260:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1291:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1293:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1293:  kernel_supports_tls is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1294:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1297:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1298:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1302:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1303:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1305:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1305:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WTF/wtf/FastMalloc.cpp:1308:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WTF/wtf/FastMalloc.cpp:1310:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1312:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WTF/wtf/FastMalloc.cpp:1313:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WTF/wtf/FastMalloc.cpp:1314:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1320:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 50 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

(In reply to comment #25)
> (In reply to comment #24)
> > (In reply to comment #23)
> > > That redefines WTF::TCMalloc_Central_FreeListPadded.
> > 
> > But but you can use a slightly different name for the templated version.
> 
> Doing this requires some reordering of the code, but it can be done.  I'm uploading a patch that adds the specialized template and reorders class definitions so everything compiles correctly with the new typedef.

oh oops, okay, then let's just use the template directly. By the way, template argument should probably be size_t instead of int.

[Alex Christensen]

> oh oops, okay, then let's just use the template directly. By the way, template argument should probably be size_t instead of int.

even using a template directly requires reordering like I did because of line 499:
class TCMalloc_Central_FreeListPadded;

which would have to be changed to 
template <int> class TCMalloc_Central_FreeListPadded;

All the pointers to TCMalloc_Central_FreeListPadded* would have to be changed to TCMalloc_Central_FreeListPadded<...sizeof...>* which means TCMalloc_Central_FreeList would have to already have been defined.

Basically there are two fixes:
1) use specialized templates for MSVC
2) reorder the whole cpp file

Which would be better?

[Darin Adler]

If you are going to move code around, then please do that in a patch that makes no changes other than moving code. You can then follow up with a patch that makes any changes needed.

[Alex Christensen]

Created attachment 148131 [details]
Patch

[WebKit Review Bot]

Attachment 148131 [details] did not pass style-queue:

Source/WTF/wtf/FastMalloc.cpp:1293:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WTF/wtf/FastMalloc.cpp:970:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WTF/wtf/FastMalloc.cpp:1114:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1116:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1118:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1118:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1119:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1120:  The parameter name "N" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1120:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1122:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1123:  The parameter name "N" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1123:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1125:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1126:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1126:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1129:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1131:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1132:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1132:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1132:  tc_length is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1135:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1138:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1139:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1140:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1154:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1158:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1159:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1160:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1160:  Use 0 or null instead of NULL (even in *comments*).  [readability/null] [4]
Source/WTF/wtf/FastMalloc.cpp:1161:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1163:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1164:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1164:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1165:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1165:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1166:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1166:  Use 0 or null instead of NULL (even in *comments*).  [readability/null] [4]
Source/WTF/wtf/FastMalloc.cpp:1167:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1169:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1170:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1171:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1172:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1174:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1175:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1176:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1177:  The parameter name "object" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1177:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1179:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1180:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1181:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1182:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1184:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1185:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1185:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1186:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1186:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1187:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1188:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1190:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1191:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1191:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1192:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1193:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1194:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1195:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1197:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1198:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1198:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1199:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1199:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1200:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1200:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1201:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1201:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1202:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1203:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1204:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1204:  locked_size_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1206:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1206:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1207:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1208:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1208:  lock_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1210:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1211:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1211:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1211:  Extra space between size_t and size_class_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1211:  size_class_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1212:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1212:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1212:  Extra space between Span and empty_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1212:  empty_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1213:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1213:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1213:  Extra space between Span and nonempty_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1213:  nonempty_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1214:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1214:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1214:  Extra space between size_t and counter_  [whitespace/declaration] [3]
Source/WTF/wtf/FastMalloc.cpp:1214:  counter_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1216:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1216:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1217:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1218:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1219:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1219:  tc_slots_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1221:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1221:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1222:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1223:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1223:  used_slots_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1224:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1224:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1225:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1226:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1227:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1227:  cache_size_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1233:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1233:  pad_ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1260:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WTF/wtf/FastMalloc.cpp:1279:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1290:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1291:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1293:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1293:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1293:  kernel_supports_tls is incoFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Fa..." exit_code: 1
rrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WTF/wtf/FastMalloc.cpp:1294:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1294:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1296:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WTF/wtf/FastMalloc.cpp:1297:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1298:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1302:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1303:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1305:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1305:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WTF/wtf/FastMalloc.cpp:1308:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WTF/wtf/FastMalloc.cpp:1310:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1312:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WTF/wtf/FastMalloc.cpp:1313:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WTF/wtf/FastMalloc.cpp:1314:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1318:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1320:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/WTF/wtf/FastMalloc.cpp:1323:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1324:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1326:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1329:  One space before end of line comments  [whitespace/comments] [5]
Source/WTF/wtf/FastMalloc.cpp:1330:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 147 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

The code doesn't pass the style checks, but all I did is put a few entire sections earlier in the code.  Should this be committed followed by another small patch adding the specialized template?  I could even do a typedef in the small patch that follows this.

[Ryosuke Niwa]

(In reply to comment #33)
> The code doesn't pass the style checks, but all I did is put a few entire sections earlier in the code.  Should this be committed followed by another small patch adding the specialized template?  I could even do a typedef in the small patch that follows this.

Please file a new bug, block this bug with that new bug, and then post the refactoring patch there.

[Alex Christensen]

> Please file a new bug, block this bug with that new bug, and then post the refactoring patch there.

Done.  Bug 89366.  It doesn't pass the style check, but all I did was move existing code.  Could somebody go review it?

[Brent Fulgham]

Created attachment 173984 [details]
Patch

[Mark Rowe (bdash)]

Comment on attachment 173984 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173984&action=review

> Source/WTF/wtf/FastMalloc.cpp:1246
> +// Zero-size specialization to avoid compiler error when TCMallo_Central_FreeList happens

"TCMallo_Central_FreeList"

[Brent Fulgham]

(In reply to comment #37)
> (From update of attachment 173984 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173984&action=review
> 
> > Source/WTF/wtf/FastMalloc.cpp:1246
> > +// Zero-size specialization to avoid compiler error when TCMallo_Central_FreeList happens
> 
> "TCMallo_Central_FreeList"

Hmm.  I'll fix that when I land it.  I'm waiting for EWS to complete a few ports first.

[Brent Fulgham]

Committed r134490: <http://trac.webkit.org/changeset/134490>

[Dean Jackson]

This has broken the Apple Lion builds.

http://build.webkit.org/builders/Apple%20Lion%20Release%20%28Build%29/builds/6429/steps/compile-webkit/logs/stdio

[Ryosuke Niwa]

(In reply to comment #40)
> This has broken the Apple Lion builds.
> 
> http://build.webkit.org/builders/Apple%20Lion%20Release%20%28Build%29/builds/6429/steps/compile-webkit/logs/stdio

Oops, fixed in r134493.

[WebKit Review Bot]

Re-opened since this is blocked by bug 102145