Add sk_calloc and sk_calloc_throw to SkMemory_new_handler.cpp.

skia/ext/SkMemory_new_handler.cpp

Index: skia/ext/SkMemory_new_handler.cpp
diff --git a/skia/ext/SkMemory_new_handler.cpp b/skia/ext/SkMemory_new_handler.cpp
index dbbc4944512a2904eeb18ed3643e854d86b77205..272d07df28328bb98e6433bc18daf0824d5df2e2 100644
--- a/skia/ext/SkMemory_new_handler.cpp
+++ b/skia/ext/SkMemory_new_handler.cpp
@@ -11,13 +11,21 @@
 #include "third_party/skia/include/core/SkTypes.h"
 #include "third_party/skia/include/core/SkThread.h"
 
-// This implementation of sk_malloc_flags() and friends is identical
-// to SkMemory_malloc.c, except that it disables the CRT's new_handler
-// during malloc(), when SK_MALLOC_THROW is not set (ie., when
-// sk_malloc_flags() would not abort on NULL).
+// This implementation of sk_malloc_flags() and friends is identical to
+// SkMemory_malloc.cpp, except that it disables the CRT's new_handler during
+// malloc() and calloc() when SK_MALLOC_THROW is not set (because our normal
+// new_handler itself will crash on failure when using tcmalloc).
 
 SK_DECLARE_STATIC_MUTEX(gSkNewHandlerMutex);
 
+static inline void* throwOnFailure(size_t size, void* p) {
+    if (size > 0 && p == NULL) {
+        // If we've got a NULL here, the only reason we should have failed is running out of RAM.
+        sk_out_of_memory();
+    }
+    return p;
+}
+
 void sk_throw() {
     SkASSERT(!"sk_throw");
     abort();
@@ -28,19 +36,8 @@ void sk_out_of_memory(void) {
     abort();
 }
 
-void* sk_malloc_throw(size_t size) {
-    return sk_malloc_flags(size, SK_MALLOC_THROW);
-}
-
 void* sk_realloc_throw(void* addr, size_t size) {
-    void* p = realloc(addr, size);
-    if (size == 0) {
-        return p;
-    }
-    if (p == NULL) {
-        sk_throw();
-    }
-    return p;
+    return throwOnFailure(size, realloc(addr, size));
 }
 
 void sk_free(void* p) {
@@ -49,29 +46,52 @@ void sk_free(void* p) {
     }
 }
 
-void* sk_malloc_flags(size_t size, unsigned flags) {
-    void* p;
+void* sk_malloc_throw(size_t size) {
+    return throwOnFailure(size, malloc(size));
+}
+
+// Platform specific ways to try really hard to get a malloc that won't crash on failure.
+static void* sk_malloc_nothrow(size_t size) {
 #if defined(ANDROID)
-    // Android doesn't have std::set_new_handler.
-    p = malloc(size);
-#else
-    if (!(flags & SK_MALLOC_THROW)) {
-#if defined(OS_MACOSX) && !defined(OS_IOS)
-      p = base::UncheckedMalloc(size);
+    // Android doesn't have std::set_new_handler, so we just call malloc.
+    return malloc(size);
+#elif defined(OS_MACOSX) && !defined(OS_IOS)
+    return base::UncheckedMalloc(size);
 #else
-      SkAutoMutexAcquire lock(gSkNewHandlerMutex);
-      std::new_handler old_handler = std::set_new_handler(NULL);
-      p = malloc(size);
-      std::set_new_handler(old_handler);
-#endif
-    } else {
-      p = malloc(size);
-    }
+    // This is not really thread safe.  It only won't collide with itself, but we're totally
+    // unprotected from races with other code that calls set_new_handler.

[Scott Hess - ex-Googler, 2013/09/24 21:05:54]

+1 for the added comment.

+    SkAutoMutexAcquire lock(gSkNewHandlerMutex);
+    std::new_handler old_handler = std::set_new_handler(NULL);
+    void* p = malloc(size);
+    std::set_new_handler(old_handler);
+    return p;
 #endif
-    if (p == NULL) {
-        if (flags & SK_MALLOC_THROW) {
-            sk_throw();
-        }
+}
+
+void* sk_malloc_flags(size_t size, unsigned flags) {
+    if (flags & SK_MALLOC_THROW) {
+        return sk_malloc_throw(size);
     }
+    return sk_malloc_nothrow(size);
+}
+
+void* sk_calloc_throw(size_t size) {
+    return throwOnFailure(size, calloc(size, 1));
+}
+
+// Jump through the same hoops as sk_malloc_nothrow to avoid a crash, but for calloc.
+void* sk_calloc(size_t size) {

[Scott Hess - ex-Googler, 2013/09/24 21:05:54]

Neither the calloc() interface, nor the sk_malloc_flags/sk_malloc_nothrow
convention.  :-).  But I see that sk_calloc is already in skia, so I guess that
ship has sailed.

[mtklein, 2013/09/24 21:12:18]

Yeah... :'(

+#if defined(ANDROID)
+    return calloc(size, 1);
+#elif defined(OS_MACOSX) && !defined(OS_IOS)
+    return base::UncheckedCalloc(size, 1);
+#else
+    SkAutoMutexAcquire lock(gSkNewHandlerMutex);
+    std::new_handler old_handler = std::set_new_handler(NULL);
+    void* p = calloc(size, 1);
+    std::set_new_handler(old_handler);
     return p;
+#endif
 }
+
+

[Scott Hess - ex-Googler, 2013/09/24 21:05:54]

Extra whitespace is extra.

[mtklein, 2013/09/24 21:12:18]

Done.