Remove gotos and just use multiple returns.

Bridge/Bridge.cpp

@@ -99,7 +99,7 @@ bridge::DeviceSystemBridge::Initialize()
     if (st != ER_OK)
     {
       DSBLOG_WARN("initialize AllJoyn failed: %s", QCC_StatusText(st));
-      goto Leave;
+      return st;
     }
 
     m_alljoynInitialized = true;
@@ -107,13 +107,7 @@ bridge::DeviceSystemBridge::Initialize()
 
   st = InitializeInternal();
   if (st != ER_OK)
-    goto Leave;
-
-Leave:
-  if (st != ER_OK)
-  {
-    Shutdown();
-  }
+    m_alljoynInitialized = false;
 
   return st;
 }
@@ -121,7 +115,7 @@ Leave:
 QStatus
 bridge::DeviceSystemBridge::InitializeInternal()
 {
-  QStatus st = ER_OK;
+  QStatus st;
 
   DSBLOG_INFO("initialize configuration manager");
   st = m_configManager.Initialize();
@@ -129,7 +123,7 @@ bridge::DeviceSystemBridge::InitializeInternal()
   if (st != ER_OK)
   {
     DSBLOG_WARN("initialize configuration manager failed: %s", QCC_StatusText(st));
-    goto Leave;
+    return st;
   }
 
   DSBLOG_INFO("initialize adapter");
@@ -138,7 +132,7 @@ bridge::DeviceSystemBridge::InitializeInternal()
   if (st != ER_OK)
   {
     DSBLOG_WARN("initialize adapter failed: %s", QCC_StatusText(st));
-    goto Leave;
+    return st;
   }
 
   DSBLOG_INFO("connect to AllJoyn router");
@@ -147,7 +141,7 @@ bridge::DeviceSystemBridge::InitializeInternal()
   if (st != ER_OK)
   {
     DSBLOG_INFO("connect to AllJoyn router failed: %s", QCC_StatusText(st));
-    goto Leave;
+    return st;
   }
 
   DSBLOG_INFO("initialize devices");
@@ -155,7 +149,7 @@ bridge::DeviceSystemBridge::InitializeInternal()
   if (st != ER_OK)
   {
     DSBLOG_WARN("initialize devices failed: %s", QCC_StatusText(st));
-    goto Leave;
+    return st;
   }
 
   DSBLOG_INFO("registering signal handlers");
@@ -163,10 +157,9 @@ bridge::DeviceSystemBridge::InitializeInternal()
   if (st != ER_OK)
   {
     DSBLOG_WARN("register signal handlers failed: %s", QCC_StatusText(st));
-    goto Leave;
+    return st;
   }
 
-Leave:
   return st;
 }
 
@@ -208,8 +201,6 @@ bridge::DeviceSystemBridge::InitializeAdapter()
 QStatus
 bridge::DeviceSystemBridge::InitializeDevices(bool update)
 {
-  QStatus st = ER_OK;
-
   AdapterDeviceVector deviceList;
   std::shared_ptr<IAdapterIoRequest> request;
 
@@ -223,8 +214,7 @@ bridge::DeviceSystemBridge::InitializeDevices(bool update)
     DSBLOG_WARN("failed to enumerate devices: %d", m_adapter->GetStatusText(adapterStatus).c_str());
 
     // TODO: we need some translation from AdapterStatus to QStatus
-    st = ER_FAIL;
-    goto Leave;
+    return ER_FAIL;
   }
 
   if (request)
@@ -233,9 +223,7 @@ bridge::DeviceSystemBridge::InitializeDevices(bool update)
     if (!IsOk(adapterStatus))
     {
       DSBLOG_WARN("failed to wait for async i/o request: %s", m_adapter->GetStatusText(adapterStatus).c_str());
-
-      st = ER_FAIL;
-      goto Leave;
+      return ER_FAIL;
     }
   }
 
@@ -243,19 +231,22 @@ bridge::DeviceSystemBridge::InitializeDevices(bool update)
   for (iterator begin = deviceList.begin(), end = deviceList.end(); begin != end; ++begin)
   {
     // TODO: get configuration for device, only expose visible devices
+    QStatus st;
     if (update)
       st = UpdateDevice(*begin, true);
     else
       st = CreateDevice(*begin);
 
     if (st != ER_OK)
+    {
       DSBLOG_WARN("Failed to %s device: %s", update ? "update" : "create", QCC_StatusText(st));
+      return st;
+    }
   }
 
   // TODO: Save bridge configuration to XML
 
-Leave:
-  return st;
+  return ER_OK;
 }
 
 void
@@ -281,20 +272,15 @@ bridge::DeviceSystemBridge::OnAdapterSignal(IAdapterSignal const& signal, void*)
       }
 
       if (!adapterDevice)
-        goto Leave;
+        return;
     }
   }
 
   if (name == kDeviceArrivalSignal)
-  {
     CreateDevice(adapterDevice);
-  }
+
   else if (name == kDeviceRemovalSignal)
-  {
     UpdateDevice(adapterDevice, false);
-  }
-Leave:
-  return;
 }
 
 QStatus

Bridge/BridgeDevice.cpp

@@ -46,17 +46,17 @@ bridge::BridgeDevice::Initialize()
   // create Device service name
   st = BuildServiceName();
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   // init alljoyn
   st = InitializeAllJoyn();
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   // initialize about service
   st = m_about.Initialize(m_busAttachment);
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   // set device info in about
   m_about.SetApplicationName(m_adapter->GetExposedApplicationName().c_str());
@@ -76,22 +76,22 @@ bridge::BridgeDevice::Initialize()
   // create device properties
   st = CreateDeviceProperties();
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   // create main device
   st = m_deviceMain.Initialize();
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   // Create a control panel if requested by the caller.
   st = InitControlPanel();
   if (st != ER_OK)
-    goto leave;
+    return st;
   
   // Create Lighting Service if requested
   st = InitLightingService();
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   interface = m_deviceMain.GetInterfaceDescription();
   if (!interface)
@@ -104,20 +104,15 @@ bridge::BridgeDevice::Initialize()
   // connect to AllJoyn
   st = ConnectToAllJoyn();
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   // register signals
   st = RegisterSignalHandlers(true);
   if (st != ER_OK)
-    goto leave;
+    return st;
 
   // announce
-  m_about.Announce();
-
-leave:
-  if (st != ER_OK)
-    Shutdown();
-  return st;
+  return m_about.Announce();
 }
 
 QStatus

Bridge/DeviceMethod.cpp

@@ -19,44 +19,19 @@ bridge::DeviceMethod::~DeviceMethod()
 }
 
 uint32_t
-bridge::DeviceMethod::InvokeMethod(ajn::Message& msg, ajn::MsgArg* outArgs, size_t* numOutArgs)
+bridge::DeviceMethod::InvokeMethod(ajn::Message& msg, std::vector<ajn::MsgArg>& outArgs)
 {
-  int       i = 0;
-  uint32_t  adapterStatus = 0;
-  QStatus   status = ER_OK;
+  int i = 0;
+  QStatus status = ER_OK;
   std::shared_ptr<IAdapterIoRequest> request;
 
-  if (outArgs)
-    *outArgs = nullptr;
-
-  if (numOutArgs)
-    *numOutArgs = 0;
-
-  if (!m_adapterMethod->GetOutputParams().empty())
-  {
-    if (outArgs == nullptr)
-      adapterStatus = 1;
-    if (numOutArgs == nullptr)
-      adapterStatus = 2;
-
-    // TODO: I don't like all these goto's. Why not just use exception handling?
-    if (adapterStatus != 0)
-      goto leave;
-
-    // TODO: make sure this gets deleted [] by caller. Should we just use vector with
-    // std::shared_ptr<>?
-    outArgs = new ajn::MsgArg[m_adapterMethod->GetOutputParams().size()];
-    *numOutArgs = m_adapterMethod->GetOutputParams().size();
-  }
+  outArgs.clear();
 
   for (std::shared_ptr<IAdapterValue> const& param : m_adapterMethod->GetInputParams())
   {
     ajn::MsgArg const* inArg = msg->GetArg(i++);
     if (inArg == nullptr)
-    {
-      adapterStatus = 1;
-      goto leave;
-    }
+      return 1;
 
     // TODO: this seems odd. The GetAdapterValue() populates the param argument with the
     // data from the inArg. Why wouldn't we create a temporary based on the incoming
@@ -66,13 +41,10 @@ bridge::DeviceMethod::InvokeMethod(ajn::Message& msg, ajn::MsgArg* outArgs, size
     // completion at any given time
     status = AllJoynHelper::GetAdapterValue(*param, *inArg);
     if (status != ER_OK)
-    {
-      adapterStatus = 1;
-      goto leave;
-    }
+      return 1;
   }
 
-  // TODO: we're ingoring the return value here
+  // TODO: we're ignoring the return value here
   bridge::DeviceSystemBridge::GetInstance()->GetAdapter()
     ->CallMethod(m_adapterMethod, &request);
 
@@ -84,18 +56,11 @@ bridge::DeviceMethod::InvokeMethod(ajn::Message& msg, ajn::MsgArg* outArgs, size
   for (std::shared_ptr<IAdapterValue> const& param : m_adapterMethod->GetOutputParams())
   {
     status = AllJoynHelper::SetMsgArg(*param, outArgs[i]);
-    // TODO: check status
+    if (status != ER_OK)
+      return 1;
   }
 
-  leave:
-    if (adapterStatus != 0)
-    {
-      delete [] outArgs;
-      if (numOutArgs)
-        *numOutArgs = 0;
-    }
-
-  return adapterStatus;
+  return 0;
 }
 
 QStatus

Bridge/DeviceMethod.h

@@ -19,7 +19,7 @@ namespace bridge
 
     // TODO: error code?
     // TODO: should that first arg be constant?
-    uint32_t InvokeMethod(ajn::Message& msg, ajn::MsgArg* outArgs, size_t* numOutArgs);
+    uint32_t InvokeMethod(ajn::Message& msg, std::vector<ajn::MsgArg>& outArgs);
 
     inline std::string const& GetName() const
       { return m_exposedName; }

DeviceProviders/AllJoynProvider.cpp

@@ -54,10 +54,10 @@ AllJoynStatus AllJoynProvider::Start()
 {
   QStatus status = ER_OK;
 
-  #define AJ_CHECK(ST) { status = ST; if (status != ER_OK) goto leave; }
+  #define AJ_CHECK(ST) { status = ST; if (status != ER_OK) return AllJoynStatus(status); }
 
   if (m_bus != NULL)
-    goto leave;
+    return AllJoynStatus(ER_FAIL);
 
   if (!m_alljoynInitialized)
   {
@@ -79,10 +79,6 @@ AllJoynStatus AllJoynProvider::Start()
 
   m_isListening = true;
 
-  leave:
-    if (status != ER_OK)
-      Shutdown();
-
   return AllJoynStatus(status);
 }