From 06881d7ca6fcec174938de86378c30b6836f31a9 Mon Sep 17 00:00:00 2001
From: Chris Manton <cmanton@google.com>
Date: Mon, 3 May 2021 09:37:29 -0700
Subject: [PATCH] Properly initialize tGATT control block

Bug: 187827452
Test: gd/cert/run
Tag: #refactor

Change-Id: Ifc867c333aac184457c6bdecca7373c6fa48b0a1
---
 stack/gatt/gatt_api.cc             |  2 +-
 stack/gatt/gatt_int.h              | 10 ++++-----
 stack/include/gatt_api.h           | 18 ++++++++--------
 stack/test/gatt/stack_gatt_test.cc | 44 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/stack/gatt/gatt_api.cc b/stack/gatt/gatt_api.cc
index 1edb9a6ce..7aa4fd4ee 100644
--- a/stack/gatt/gatt_api.cc
+++ b/stack/gatt/gatt_api.cc
@@ -1087,7 +1087,7 @@ void GATT_Deregister(tGATT_IF gatt_if) {
 
   connection_manager::on_app_deregistered(gatt_if);
 
-  memset(p_reg, 0, sizeof(tGATT_REG));
+  *p_reg = {};
 }
 
 /*******************************************************************************
diff --git a/stack/gatt/gatt_int.h b/stack/gatt/gatt_int.h
index 1a5980e24..cb9742fb9 100644
--- a/stack/gatt/gatt_int.h
+++ b/stack/gatt/gatt_int.h
@@ -188,11 +188,11 @@ typedef struct {
 
 typedef struct {
   bluetooth::Uuid app_uuid128;
-  tGATT_CBACK app_cb;
-  tGATT_IF gatt_if; /* one based */
-  bool in_use;
-  uint8_t listening; /* if adv for all has been enabled */
-  bool eatt_support;
+  tGATT_CBACK app_cb{};
+  tGATT_IF gatt_if{0}; /* one based */
+  bool in_use{false};
+  uint8_t listening{0}; /* if adv for all has been enabled */
+  bool eatt_support{false};
 } tGATT_REG;
 
 struct tGATT_CLCB;
diff --git a/stack/include/gatt_api.h b/stack/include/gatt_api.h
index 15948ab91..b79092aa1 100644
--- a/stack/include/gatt_api.h
+++ b/stack/include/gatt_api.h
@@ -684,15 +684,15 @@ typedef void(tGATT_CONN_UPDATE_CB)(tGATT_IF gatt_if, uint16_t conn_id,
  * MUST be provided.
 */
 typedef struct {
-  tGATT_CONN_CBACK* p_conn_cb;
-  tGATT_CMPL_CBACK* p_cmpl_cb;
-  tGATT_DISC_RES_CB* p_disc_res_cb;
-  tGATT_DISC_CMPL_CB* p_disc_cmpl_cb;
-  tGATT_REQ_CBACK* p_req_cb;
-  tGATT_ENC_CMPL_CB* p_enc_cmpl_cb;
-  tGATT_CONGESTION_CBACK* p_congestion_cb;
-  tGATT_PHY_UPDATE_CB* p_phy_update_cb;
-  tGATT_CONN_UPDATE_CB* p_conn_update_cb;
+  tGATT_CONN_CBACK* p_conn_cb{nullptr};
+  tGATT_CMPL_CBACK* p_cmpl_cb{nullptr};
+  tGATT_DISC_RES_CB* p_disc_res_cb{nullptr};
+  tGATT_DISC_CMPL_CB* p_disc_cmpl_cb{nullptr};
+  tGATT_REQ_CBACK* p_req_cb{nullptr};
+  tGATT_ENC_CMPL_CB* p_enc_cmpl_cb{nullptr};
+  tGATT_CONGESTION_CBACK* p_congestion_cb{nullptr};
+  tGATT_PHY_UPDATE_CB* p_phy_update_cb{nullptr};
+  tGATT_CONN_UPDATE_CB* p_conn_update_cb{nullptr};
 } tGATT_CBACK;
 
 /*****************  Start Handle Management Definitions   *********************/
diff --git a/stack/test/gatt/stack_gatt_test.cc b/stack/test/gatt/stack_gatt_test.cc
index 7fca4cb71..86a697d27 100644
--- a/stack/test/gatt/stack_gatt_test.cc
+++ b/stack/test/gatt/stack_gatt_test.cc
@@ -15,11 +15,15 @@
  */
 
 #include <gtest/gtest.h>
+#include <string.h>
+
 #include <cstdint>
 #include <map>
+#include <memory>
 #include <string>
 
 #include "common/message_loop_thread.h"
+#include "stack/gatt/gatt_int.h"
 
 std::map<std::string, int> mock_function_count_map;
 
@@ -29,4 +33,42 @@ bluetooth::common::MessageLoopThread* get_main_thread() { return nullptr; }
 
 class StackGattTest : public ::testing::Test {};
 
-TEST_F(StackGattTest, nop) {}
+// Actual size of structure without compiler padding
+size_t actual_sizeof_tGATT_REG() {
+  return sizeof(bluetooth::Uuid) + sizeof(tGATT_CBACK) + sizeof(tGATT_IF) +
+         sizeof(bool) + sizeof(uint8_t) + sizeof(bool);
+}
+
+TEST_F(StackGattTest, lifecycle_tGATT_REG) {
+  {
+    std::unique_ptr<tGATT_REG> reg0 = std::make_unique<tGATT_REG>();
+    std::unique_ptr<tGATT_REG> reg1 = std::make_unique<tGATT_REG>();
+    memset(reg0.get(), 0xff, sizeof(tGATT_REG));
+    memset(reg1.get(), 0xff, sizeof(tGATT_REG));
+    ASSERT_EQ(0, memcmp(reg0.get(), reg1.get(), sizeof(tGATT_REG)));
+  }
+
+  {
+    std::unique_ptr<tGATT_REG> reg0 = std::make_unique<tGATT_REG>();
+    memset(reg0.get(), 0xff, sizeof(tGATT_REG));
+
+    tGATT_REG reg1;
+    memset(&reg1, 0xff, sizeof(tGATT_REG));
+
+    // Clear the structures
+    memset(reg0.get(), 0, sizeof(tGATT_REG));
+    reg1 = {};
+    ASSERT_EQ(0, memcmp(reg0.get(), &reg1, actual_sizeof_tGATT_REG()));
+  }
+
+  {
+    tGATT_REG* reg0 = new tGATT_REG();
+    tGATT_REG* reg1 = new tGATT_REG();
+    memset(reg0, 0, sizeof(tGATT_REG));
+    *reg1 = {};
+    reg0->in_use = true;
+    ASSERT_NE(0, memcmp(reg0, reg1, sizeof(tGATT_REG)));
+    delete reg1;
+    delete reg0;
+  }
+}
-- 
2.11.0