From cac38f92dd8dbba0d6ac8886aa5d9f9c9e583386 Mon Sep 17 00:00:00 2001
From: David Neto <dneto@google.com>
Date: Tue, 8 Sep 2015 15:29:22 -0400
Subject: [PATCH] Simplify uses of spvBinaryDestroy in tests

Always try to destroy the binary during common methods
of test fixtures.  This is safe if no other code in the test
attempted to destroy the binary.

Take advantage of the fact spvBinaryDestroy is a no-op on a nullptr,
by eliminating the null pointer check in the caller.
---
 test/Comment.cpp      |  3 ---
 test/ImmediateInt.cpp |  2 --
 test/NamedId.cpp      |  2 +-
 test/TestFixture.h    | 10 +++++++++-
 test/TextToBinary.cpp | 10 ----------
 5 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/test/Comment.cpp b/test/Comment.cpp
index 401f2fa..4974fd4 100644
--- a/test/Comment.cpp
+++ b/test/Comment.cpp
@@ -42,9 +42,6 @@ TEST_F(TextToBinaryTest, Whitespace) {
 )");
   EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable,
                                          extInstTable, &binary, &diagnostic));
-  if (binary) {
-    spvBinaryDestroy(binary);
-  }
   if (diagnostic) {
     spvDiagnosticPrint(diagnostic);
   }
diff --git a/test/ImmediateInt.cpp b/test/ImmediateInt.cpp
index 9d6f603..3ed39a2 100644
--- a/test/ImmediateInt.cpp
+++ b/test/ImmediateInt.cpp
@@ -42,7 +42,6 @@ TEST_F(TextToBinaryTest, ImmediateIntOpCode) {
   ASSERT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable,
                                          extInstTable, &binary, &diagnostic));
   EXPECT_EQ(0x00FF00FF, binary->code[5]);
-  spvBinaryDestroy(binary);
   if (diagnostic) {
     spvDiagnosticPrint(diagnostic);
   }
@@ -53,7 +52,6 @@ TEST_F(TextToBinaryTest, ImmediateIntOperand) {
   EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable,
                                          extInstTable, &binary, &diagnostic));
   EXPECT_EQ(0x00FF00FF, binary->code[6]);
-  spvBinaryDestroy(binary);
   if (diagnostic) {
     spvDiagnosticPrint(diagnostic);
   }
diff --git a/test/NamedId.cpp b/test/NamedId.cpp
index 144b63c..8a452ae 100644
--- a/test/NamedId.cpp
+++ b/test/NamedId.cpp
@@ -48,7 +48,7 @@ TEST(NamedId, Default) {
   ASSERT_EQ(SPV_SUCCESS, spvOperandTableGet(&operandTable));
   spv_ext_inst_table extInstTable;
   ASSERT_EQ(SPV_SUCCESS, spvExtInstTableGet(&extInstTable));
-  spv_binary binary;
+  spv_binary binary = nullptr;
   spv_diagnostic diagnostic;
   spv_result_t error = spvTextToBinary(&text, opcodeTable, operandTable,
                                        extInstTable, &binary, &diagnostic);
diff --git a/test/TestFixture.h b/test/TestFixture.h
index f91e694..e7c4cb8 100644
--- a/test/TestFixture.h
+++ b/test/TestFixture.h
@@ -55,6 +55,7 @@ class TextToBinaryTestBase : public T {
   }
 
   virtual ~TextToBinaryTestBase() {
+    DestroyBinary();
     if (diagnostic) spvDiagnosticDestroy(diagnostic);
   }
 
@@ -75,7 +76,7 @@ class TextToBinaryTestBase : public T {
     SpirvVector code_copy;
     if (status == SPV_SUCCESS) {
       code_copy = SpirvVector(binary->code, binary->code + binary->wordCount);
-      spvBinaryDestroy(binary);
+      DestroyBinary();
     } else {
       spvDiagnosticPrint(diagnostic);
     }
@@ -90,6 +91,7 @@ class TextToBinaryTestBase : public T {
               spvTextToBinary(&this->text, opcodeTable, operandTable,
                               extInstTable, &binary, &diagnostic))
         << text;
+    DestroyBinary();
     return diagnostic->error;
   }
 
@@ -99,6 +101,12 @@ class TextToBinaryTestBase : public T {
     text.length = textString.size();
   }
 
+  // Destroys the binary, if it exists.
+  void DestroyBinary() {
+    spvBinaryDestroy(binary);
+    binary = nullptr;
+  }
+
   spv_opcode_table opcodeTable;
   spv_operand_table operandTable;
   spv_ext_inst_table extInstTable;
diff --git a/test/TextToBinary.cpp b/test/TextToBinary.cpp
index 7152430..16e02c3 100644
--- a/test/TextToBinary.cpp
+++ b/test/TextToBinary.cpp
@@ -255,9 +255,6 @@ TEST_F(TextToBinaryTest, StringSpace) {
   SetText("OpSourceExtension \"string with spaces\"");
   EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable,
                                          extInstTable, &binary, &diagnostic));
-  if (binary) {
-    spvBinaryDestroy(binary);
-  }
   if (diagnostic) {
     spvDiagnosticPrint(diagnostic);
   }
@@ -288,9 +285,6 @@ TEST_F(TextToBinaryTest, InstructionTwoFormats) {
 
   EXPECT_EQ(SPV_SUCCESS, spvTextToBinary(&text, opcodeTable, operandTable,
                                          extInstTable, &binary, &diagnostic));
-  if (binary) {
-    spvBinaryDestroy(binary);
-  }
   if (diagnostic) {
     spvDiagnosticPrint(diagnostic);
   }
@@ -312,7 +306,6 @@ Google
       "Expected <opcode> or <result-id> at the beginning of an instruction, "
       "found 'Google'.",
       diagnostic->error);
-  if (binary) spvBinaryDestroy(binary);
 }
 
 TEST_F(TextToBinaryTest, NoEqualSign) {
@@ -328,7 +321,6 @@ TEST_F(TextToBinaryTest, NoEqualSign) {
   EXPECT_EQ(5, diagnostic->position.line + 1);
   EXPECT_EQ(1, diagnostic->position.column + 1);
   EXPECT_STREQ("Expected '=', found end of stream.", diagnostic->error);
-  if (binary) spvBinaryDestroy(binary);
 }
 
 TEST_F(TextToBinaryTest, NoOpCode) {
@@ -344,7 +336,6 @@ TEST_F(TextToBinaryTest, NoOpCode) {
   EXPECT_EQ(5, diagnostic->position.line + 1);
   EXPECT_EQ(1, diagnostic->position.column + 1);
   EXPECT_STREQ("Expected opcode, found end of stream.", diagnostic->error);
-  if (binary) spvBinaryDestroy(binary);
 }
 
 TEST_F(TextToBinaryTest, WrongOpCode) {
@@ -360,7 +351,6 @@ TEST_F(TextToBinaryTest, WrongOpCode) {
   EXPECT_EQ(4, diagnostic->position.line + 1);
   EXPECT_EQ(6, diagnostic->position.column + 1);
   EXPECT_STREQ("Invalid Opcode prefix 'Wahahaha'.", diagnostic->error);
-  if (binary) spvBinaryDestroy(binary);
 }
 
 TEST_F(TextToBinaryTest, GoodSwitch) {
-- 
2.7.4