[Webkit-unassigned] [Bug 89608] FastMalloc.cpp needs to have its style updated
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 20 19:00:35 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=89608
--- Comment #4 from Ryosuke Niwa <rniwa at webkit.org> 2012-06-20 19:00:35 PST ---
(From update of attachment 148670)
View in context: https://bugs.webkit.org/attachment.cgi?id=148670&action=review
> Source/WTF/wtf/FastMalloc.cpp:1409
> + PageMap pagemap;
> + mutable PageMapCache pagemapcache;
pageMap, pageMapCache.
> Source/WTF/wtf/FastMalloc.cpp:1727
> + } else
> + // Keep looking in larger classes
> + continue;
> +
These two lines should have curly brackets.
> Source/WTF/wtf/FastMalloc.cpp:1768
> + for (Span* span = large.normal.next;
> + span != &large.normal;
> + span = span->next) {
It seems like this can all fit in one line.
> Source/WTF/wtf/FastMalloc.cpp:1782
> + for (Span* span = large.returned.next;
> + span != &large.returned;
> + span = span->next) {
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:1995
> + static_cast<size_t>(s->length << kPageShift));
Wrong indentation. It should be indented 4 spaces to the right of TCMalloc_SystemRelease.
(i.e. we don't align subsequent indentations).
> Source/WTF/wtf/FastMalloc.cpp:2230
> + static_cast<size_t>(s->length << kPageShift));
Ditto about wrong indentation.
> Source/WTF/wtf/FastMalloc.cpp:2297
> + void PushRange(int N, void *start, void *end)
Nit: void* instead of void *.
> Source/WTF/wtf/FastMalloc.cpp:2303
> + void PopRange(int N, void **start, void **end)
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:2370
> + int freelistlength(size_t cl) const
> + {
> + return list[cl].length();
> + }
You can keep this a one-line function. We allow f() { return ~ }. In fact, it's probably preferable here because the definition is so trivial.
> Source/WTF/wtf/FastMalloc.cpp:2379
> + void Deallocate(void* ptr, size_t sizeclass);
Useless argument name ptr.
> Source/WTF/wtf/FastMalloc.cpp:2388
> + bool SampleAllocation(size_t k);
Ditto about k.
> Source/WTF/wtf/FastMalloc.cpp:2391
> + void PickNextSample(size_t k);
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:2425
> + void InsertRange(void *start, void *end, int);
Nit: void* instead of void *.
> Source/WTF/wtf/FastMalloc.cpp:2428
> + void RemoveRange(void **start, void **end, int*);
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:2962
> + ptr = nptr;
Nit: extra space.
> Source/WTF/wtf/FastMalloc.cpp:3011
> + for (size_t cl = 0; cl < kNumClasses; ++cl)
> + if (list[cl].length() > 0)
> + ReleaseToCentralCache(cl, list[cl].length());
This "for" needs curly brackets because it contains two lines.
> Source/WTF/wtf/FastMalloc.cpp:3113
> + // increment is "sample_period/2".
Nit: spaces around /.
> Source/WTF/wtf/FastMalloc.cpp:3122
> + for (i = 0; i < (static_cast<int>(sizeof(primesList) / sizeof(primesList[0])) - 1); i++)
> + if (primesList[i] >= flagValue)
> + break;
Ditto about curly brackets.
> Source/WTF/wtf/FastMalloc.cpp:3332
> + if (GetThreadHeap() == heap)
> + // Somehow heap got reinstated by a recursive call to malloc
> + // from pthread_setspecific. We give up in this case.
> + return;
We need to keep curly brackets for this. In WebKit style, we decide whether we use curly brackets or not based on the number of physical lines, not the number of statements.
> Source/WTF/wtf/FastMalloc.cpp:3429
> + if (classCount)
> + for (size_t cl = 0; cl < kNumClasses; ++cl)
> + classCount[cl] += h->freelistlength(cl);
Ditto about curly brackets.
> Source/WTF/wtf/FastMalloc.cpp:3579
> + - stats.threadBytes
> + - stats.centralBytes
> + - stats.pageheapBytes;
Wrong indentation. "- stats." should be 4 spaces to the right of "*value".
> Source/WTF/wtf/FastMalloc.cpp:3593
> + SpinLockHolder ll(&pageheapLock);
Why did you rename l to ll?
> Source/WTF/wtf/FastMalloc.cpp:3625
> + SpinLockHolder ll(&pageheapLock);
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:3785
> + } else
> + // The common case, and also the simplest. This just pops the
> + // size-appropriate freelist, afer replenishing it if it's empty.
> + ret = CheckedMallocResult(heap->Allocate(size));
Keep the curly brackets here as well.
> Source/WTF/wtf/FastMalloc.cpp:-3687
> + if (!ret)
> #ifdef WTF_CHANGES
> - if (crashOnFailure) // This branch should be optimized out by the compiler.
> - CRASH();
> + if (crashOnFailure) // This branch should be optimized out by the compiler.
> + CRASH();
> #else
> - errno = ENOMEM;
> + errno = ENOMEM;
> #endif
> - }
> - return ret;
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:3864
> + cl++;
Missing indentation.
> Source/WTF/wtf/FastMalloc.cpp:3939
> + info.fsmblks = static_cast<int>(stats.threadBytes
> + + stats.centralBytes
> + + stats.transferBytes);
Wrong indentation.
> Source/WTF/wtf/FastMalloc.cpp:3946
> + - stats.threadBytes
> + - stats.centralBytes
> + - stats.transferBytes
> + - stats.pageheapBytes);
> +
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:4033
> +void* fastCalloc(size_t n, size_t elemsize)
Should be elemSize instead.
> Source/WTF/wtf/FastMalloc.cpp:4042
> +TryMallocReturnValue tryFastCalloc(size_t n, size_t elemsize)
Dotto.
> Source/WTF/wtf/FastMalloc.cpp:4054
> +void* calloc(size_t n, size_t elemsize)
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:4100
> +void* fastRealloc(void* oldptr, size_t newSize)
Nit: s/oldptr/oldPtr/
> Source/WTF/wtf/FastMalloc.cpp:4112
> +TryMallocReturnValue tryFastRealloc(void* oldptr, size_t newSize)
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:4127
> +void* realloc(void* oldptr, size_t newSize)
Ditto.
> Source/WTF/wtf/FastMalloc.cpp:4179
> + void* newptr = doMalloc(newSize);
Nit: s/newptr/newPtr/
> Source/WTF/wtf/FastMalloc.cpp:4315
> +extern "C" int posix_memalign(void** resultptr, size_t align, size_t size)
Nit: s/resultptr/resultPtr/
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list