From dee9d81da59a04fa061cabdcccd264620d22565e Mon Sep 17 00:00:00 2001
From: Andreas Schiffler <aschiffler@ferzkopp.net>
Date: Sat, 4 May 2013 09:11:18 -0700
Subject: [PATCH] Added additional input validation to SDL_AllocPalette; added
 error codes; added test case to Pixels suite for coverage

---
 src/video/SDL_pixels.c       |  9 ++++
 test/testautomation_pixels.c | 89 +++++++++++++++++++++++++++++++++++-
 2 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/src/video/SDL_pixels.c b/src/video/SDL_pixels.c
index 84a75b8eb..55e181922 100644
--- a/src/video/SDL_pixels.c
+++ b/src/video/SDL_pixels.c
@@ -503,6 +503,7 @@ SDL_AllocFormat(Uint32 pixel_format)
     }
     if (SDL_InitFormat(format, pixel_format) < 0) {
         SDL_free(format);
+        SDL_InvalidParamError("format");
         return NULL;
     }
 
@@ -585,6 +586,7 @@ SDL_FreeFormat(SDL_PixelFormat *format)
     SDL_PixelFormat *prev;
 
     if (!format) {
+        SDL_InvalidParamError("format");
         return;
     }
     if (--format->refcount > 0) {
@@ -613,6 +615,12 @@ SDL_Palette *
 SDL_AllocPalette(int ncolors)
 {
     SDL_Palette *palette;
+    
+    /* Input validation */
+    if (ncolors < 1) {
+      SDL_InvalidParamError("ncolors");
+      return NULL;
+    }
 
     palette = (SDL_Palette *) SDL_malloc(sizeof(*palette));
     if (!palette) {
@@ -693,6 +701,7 @@ void
 SDL_FreePalette(SDL_Palette * palette)
 {
     if (!palette) {
+        SDL_InvalidParamError("palette");
         return;
     }
     if (--palette->refcount > 0) {
diff --git a/test/testautomation_pixels.c b/test/testautomation_pixels.c
index 73cf12460..0165e73f7 100644
--- a/test/testautomation_pixels.c
+++ b/test/testautomation_pixels.c
@@ -101,6 +101,9 @@ char* _nonRGBPixelFormatsVerbose[] =
 
 /**
  * @brief Call to SDL_AllocFormat and SDL_FreeFormat
+ *
+ * @sa http://wiki.libsdl.org/moin.fcg/SDL_AllocFormat
+ * @sa http://wiki.libsdl.org/moin.fcg/SDL_FreeFormat 
  */
 int
 pixels_allocFreeFormat(void *arg)
@@ -145,18 +148,100 @@ pixels_allocFreeFormat(void *arg)
     SDLTest_AssertCheck(result == NULL, "Verify result is NULL");  
   }
   
+  /* Negative cases */
+  format = 0xffffffff;
+  result = SDL_AllocFormat(format);
+  SDLTest_AssertPass("Call to SDL_AllocFormat(0xffffffff)");
+  SDLTest_AssertCheck(result == NULL, "Verify result is NULL");  
+  /* TODO: check error code */
+
+  SDL_FreeFormat(NULL);
+  SDLTest_AssertPass("Call to SDL_FreeFormat(NULL)");
+  /* TODO: check error code */
+  
+  return TEST_COMPLETED;
+}
+
+/**
+ * @brief Call to SDL_AllocPalette and SDL_FreePalette
+ *
+ * @sa http://wiki.libsdl.org/moin.fcg/SDL_AllocPalette
+ * @sa http://wiki.libsdl.org/moin.fcg/SDL_FreePalette
+ */
+int
+pixels_allocFreePalette(void *arg)
+{
+  int variation;
+  int i;
+  int ncolors;
+  SDL_Palette* result;
+
+  /* Allocate palette */
+  for (variation = 1; variation <= 3; variation++) {  
+    switch (variation) {
+      /* Just one color */
+      case 1:
+        ncolors = 1;
+        break;
+      /* Two colors */
+      case 2:
+        ncolors = 2;
+        break;
+      /* More than two colors */
+      case 3:
+        ncolors = SDLTest_RandomIntegerInRange(8, 16);
+        break;
+    }
+
+    result = SDL_AllocPalette(ncolors);
+    SDLTest_AssertPass("Call to SDL_AllocPalette(%d)", ncolors);
+    SDLTest_AssertCheck(result != NULL, "Verify result is not NULL");  
+    if (result != NULL) {
+      SDLTest_AssertCheck(result->ncolors == ncolors, "Verify value of result.ncolors; expected: %u, got %u", ncolors, result->ncolors);  
+      if (result->ncolors > 0) {
+        SDLTest_AssertCheck(result->colors != NULL, "Verify value of result.colors is not NULL");
+        if (result->colors != NULL) {
+          for(i = 0; i < result->ncolors; i++) {
+            SDLTest_AssertCheck(result->colors[i].r == 255, "Verify value of result.colors[%d].r; expected: 255, got %u", i, result->colors[i].r);  
+            SDLTest_AssertCheck(result->colors[i].g == 255, "Verify value of result.colors[%d].g; expected: 255, got %u", i, result->colors[i].g);  
+            SDLTest_AssertCheck(result->colors[i].b == 255, "Verify value of result.colors[%d].b; expected: 255, got %u", i, result->colors[i].b);  
+           }
+         } 
+      }
+      
+      /* Deallocate again */
+      SDL_FreePalette(result);
+      SDLTest_AssertPass("Call to SDL_FreePalette()");          
+    }
+  }
+
+  /* Negative cases */
+  for (ncolors = 0; ncolors > -3; ncolors--) {
+    result = SDL_AllocPalette(ncolors);
+    SDLTest_AssertPass("Call to SDL_AllocPalette(%d)", ncolors);
+    SDLTest_AssertCheck(result == NULL, "Verify result is NULL");
+    /* TODO: check error code */
+  }
+
+  SDL_FreePalette(NULL);
+  SDLTest_AssertPass("Call to SDL_FreePalette(NULL)");
+  /* TODO: check error code */
+    
   return TEST_COMPLETED;
 }
 
 /* ================= Test References ================== */
 
-/* SysWM test cases */
+/* Pixels test cases */
 static const SDLTest_TestCaseReference pixelsTest1 =
 		{ (SDLTest_TestCaseFp)pixels_allocFreeFormat, "pixels_allocFreeFormat", "Call to SDL_AllocFormat and SDL_FreeFormat", TEST_ENABLED };
 
+static const SDLTest_TestCaseReference pixelsTest2 =
+		{ (SDLTest_TestCaseFp)pixels_allocFreePalette, "pixels_allocFreePalette", "Call to SDL_AllocPalette and SDL_FreePalette", TEST_ENABLED };
+
 /* Sequence of Pixels test cases */
 static const SDLTest_TestCaseReference *pixelsTests[] =  {
-	&pixelsTest1, NULL
+	&pixelsTest1, &pixelsTest2, NULL
 };
 
 /* Pixels test suite (global) */