From b1ac8a4f197da3ba3677d368574538f188de3832 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 11 Apr 2019 10:11:59 -0700 Subject: [PATCH] [joiner] remove static requirement for vendor parameters in Start() (#3748) This commit updates the `Joiner` class implementation to remove the requirement for `Start()` vendor related parameters (vendor name, model, sw version, data) to be static (string). This is done without requiring the `Joiner` class to use member variables to store/copy the strings by preparing the "Joiner Finalize" message early on (note that these parameters are included in this message) and keeping track of the dynamically allocated `Coap::Message` pointer instead. --- include/openthread/joiner.h | 8 +-- src/core/meshcop/joiner.cpp | 102 ++++++++++++++++++++++-------------- src/core/meshcop/joiner.hpp | 20 ++++--- 3 files changed, 78 insertions(+), 52 deletions(-) diff --git a/include/openthread/joiner.h b/include/openthread/joiner.h index ed144eaeb..35ba19c31 100644 --- a/include/openthread/joiner.h +++ b/include/openthread/joiner.h @@ -84,10 +84,10 @@ typedef void(OTCALL *otJoinerCallback)(otError aError, void *aContext); * @param[in] aInstance A pointer to an OpenThread instance. * @param[in] aPSKd A pointer to the PSKd. * @param[in] aProvisioningUrl A pointer to the Provisioning URL (may be NULL). - * @param[in] aVendorName A pointer to the Vendor Name (must be static). - * @param[in] aVendorModel A pointer to the Vendor Model (must be static). - * @param[in] aVendorSwVersion A pointer to the Vendor SW Version (must be static). - * @param[in] aVendorData A pointer to the Vendor Data (must be static). + * @param[in] aVendorName A pointer to the Vendor Name (may be NULL). + * @param[in] aVendorModel A pointer to the Vendor Model (may be NULL). + * @param[in] aVendorSwVersion A pointer to the Vendor SW Version (may be NULL). + * @param[in] aVendorData A pointer to the Vendor Data (may be NULL). * @param[in] aCallback A pointer to a function that is called when the join operation completes. * @param[in] aContext A pointer to application-specific context. * diff --git a/src/core/meshcop/joiner.cpp b/src/core/meshcop/joiner.cpp index c9b58c140..6174af5c6 100644 --- a/src/core/meshcop/joiner.cpp +++ b/src/core/meshcop/joiner.cpp @@ -63,10 +63,7 @@ Joiner::Joiner(Instance &aInstance) , mCallback(NULL) , mContext(NULL) , mJoinerRouterIndex(0) - , mVendorName(NULL) - , mVendorModel(NULL) - , mVendorSwVersion(NULL) - , mVendorData(NULL) + , mFinalizeMessage(NULL) , mTimer(aInstance, &Joiner::HandleTimer, this) , mJoinerEntrust(OT_URI_PATH_JOINER_ENTRUST, &Joiner::HandleJoinerEntrust, this) { @@ -116,22 +113,20 @@ otError Joiner::Start(const char * aPSKd, SuccessOrExit(error = Get().Start(kJoinerUdpPort)); SuccessOrExit(error = Get().SetPsk(reinterpret_cast(aPSKd), static_cast(strlen(aPSKd)))); - SuccessOrExit(error = Get().GetDtls().mProvisioningUrl.SetProvisioningUrl(aProvisioningUrl)); for (JoinerRouter *router = &mJoinerRouters[0]; router < OT_ARRAY_END(mJoinerRouters); router++) { router->mPriority = 0; // Priority zero means entry is not in-use. } + SuccessOrExit(error = PrepareJoinerFinalizeMessage(aProvisioningUrl, aVendorName, aVendorModel, aVendorSwVersion, + aVendorData)); + SuccessOrExit(error = Get().Discover(Mac::ChannelMask(0), Get().GetPanId(), /* aJoiner */ true, /* aEnableFiltering */ true, HandleDiscoverResult, this)); - mVendorName = aVendorName; - mVendorModel = aVendorModel; - mVendorSwVersion = aVendorSwVersion; - mVendorData = aVendorData; - mCallback = aCallback; - mContext = aContext; + mCallback = aCallback; + mContext = aContext; SetState(OT_JOINER_STATE_DISCOVER); @@ -139,6 +134,7 @@ exit: if (error != OT_ERROR_NONE) { otLogInfoMeshCoP("Error %s while starting joiner", otThreadErrorToString(error)); + FreeJoinerFinalizeMessage(); } return error; @@ -172,6 +168,7 @@ void Joiner::Finish(otError aError) case OT_JOINER_STATE_DISCOVER: Get().Stop(); + FreeJoinerFinalizeMessage(); break; } @@ -388,74 +385,99 @@ exit: return; } -void Joiner::SendJoinerFinalize(void) +otError Joiner::PrepareJoinerFinalizeMessage(const char *aProvisioningUrl, + const char *aVendorName, + const char *aVendorModel, + const char *aVendorSwVersion, + const char *aVendorData) { - otError error = OT_ERROR_NONE; - Coap::Message * message = NULL; + otError error = OT_ERROR_NONE; StateTlv stateTlv; VendorNameTlv vendorNameTlv; VendorModelTlv vendorModelTlv; VendorSwVersionTlv vendorSwVersionTlv; VendorStackVersionTlv vendorStackVersionTlv; + ProvisioningUrlTlv provisioningUrlTlv; - VerifyOrExit((message = NewMeshCoPMessage(Get())) != NULL, error = OT_ERROR_NO_BUFS); + VerifyOrExit((mFinalizeMessage = NewMeshCoPMessage(Get())) != NULL, error = OT_ERROR_NO_BUFS); - message->Init(OT_COAP_TYPE_CONFIRMABLE, OT_COAP_CODE_POST); - message->AppendUriPathOptions(OT_URI_PATH_JOINER_FINALIZE); - message->SetPayloadMarker(); - message->SetOffset(message->GetLength()); + mFinalizeMessage->Init(OT_COAP_TYPE_CONFIRMABLE, OT_COAP_CODE_POST); + mFinalizeMessage->AppendUriPathOptions(OT_URI_PATH_JOINER_FINALIZE); + mFinalizeMessage->SetPayloadMarker(); + mFinalizeMessage->SetOffset(mFinalizeMessage->GetLength()); stateTlv.Init(); stateTlv.SetState(MeshCoP::StateTlv::kAccept); - SuccessOrExit(error = message->Append(&stateTlv, sizeof(stateTlv))); + SuccessOrExit(error = mFinalizeMessage->Append(&stateTlv, sizeof(stateTlv))); vendorNameTlv.Init(); - vendorNameTlv.SetVendorName(mVendorName); - SuccessOrExit(error = message->Append(&vendorNameTlv, vendorNameTlv.GetSize())); + vendorNameTlv.SetVendorName(aVendorName); + SuccessOrExit(error = mFinalizeMessage->Append(&vendorNameTlv, vendorNameTlv.GetSize())); vendorModelTlv.Init(); - vendorModelTlv.SetVendorModel(mVendorModel); - SuccessOrExit(error = message->Append(&vendorModelTlv, vendorModelTlv.GetSize())); + vendorModelTlv.SetVendorModel(aVendorModel); + SuccessOrExit(error = mFinalizeMessage->Append(&vendorModelTlv, vendorModelTlv.GetSize())); vendorSwVersionTlv.Init(); - vendorSwVersionTlv.SetVendorSwVersion(mVendorSwVersion); - SuccessOrExit(error = message->Append(&vendorSwVersionTlv, vendorSwVersionTlv.GetSize())); + vendorSwVersionTlv.SetVendorSwVersion(aVendorSwVersion); + SuccessOrExit(error = mFinalizeMessage->Append(&vendorSwVersionTlv, vendorSwVersionTlv.GetSize())); vendorStackVersionTlv.Init(); vendorStackVersionTlv.SetOui(OPENTHREAD_CONFIG_STACK_VENDOR_OUI); vendorStackVersionTlv.SetMajor(OPENTHREAD_CONFIG_STACK_VERSION_MAJOR); vendorStackVersionTlv.SetMinor(OPENTHREAD_CONFIG_STACK_VERSION_MINOR); vendorStackVersionTlv.SetRevision(OPENTHREAD_CONFIG_STACK_VERSION_REV); - SuccessOrExit(error = message->Append(&vendorStackVersionTlv, vendorStackVersionTlv.GetSize())); + SuccessOrExit(error = mFinalizeMessage->Append(&vendorStackVersionTlv, vendorStackVersionTlv.GetSize())); - if (mVendorData != NULL) + if (aVendorData != NULL) { VendorDataTlv vendorDataTlv; vendorDataTlv.Init(); - vendorDataTlv.SetVendorData(mVendorData); - SuccessOrExit(error = message->Append(&vendorDataTlv, vendorDataTlv.GetSize())); + vendorDataTlv.SetVendorData(aVendorData); + SuccessOrExit(error = mFinalizeMessage->Append(&vendorDataTlv, vendorDataTlv.GetSize())); } - if (Get().GetDtls().mProvisioningUrl.GetLength() > 0) + provisioningUrlTlv.Init(); + provisioningUrlTlv.SetProvisioningUrl(aProvisioningUrl); + + if (provisioningUrlTlv.GetLength() > 0) { - SuccessOrExit(error = message->Append(&Get().GetDtls().mProvisioningUrl, - Get().GetDtls().mProvisioningUrl.GetSize())); + SuccessOrExit(error = mFinalizeMessage->Append(&provisioningUrlTlv, provisioningUrlTlv.GetSize())); } +exit: + if (error != OT_ERROR_NONE) + { + FreeJoinerFinalizeMessage(); + } + + return error; +} + +void Joiner::FreeJoinerFinalizeMessage(void) +{ + if (mFinalizeMessage != NULL) + { + mFinalizeMessage->Free(); + mFinalizeMessage = NULL; + } +} + +void Joiner::SendJoinerFinalize(void) +{ + assert(mFinalizeMessage != NULL); + #if OPENTHREAD_ENABLE_CERT_LOG - LogCertMessage("[THCI] direction=send | type=JOIN_FIN.req |", *message); + LogCertMessage("[THCI] direction=send | type=JOIN_FIN.req |", *mFinalizeMessage); #endif - SuccessOrExit(error = Get().SendMessage(*message, Joiner::HandleJoinerFinalizeResponse, this)); + SuccessOrExit(Get().SendMessage(*mFinalizeMessage, Joiner::HandleJoinerFinalizeResponse, this)); + mFinalizeMessage = NULL; otLogInfoMeshCoP("Joiner sent finalize"); exit: - - if (error != OT_ERROR_NONE && message != NULL) - { - message->Free(); - } + return; } void Joiner::HandleJoinerFinalizeResponse(void * aContext, diff --git a/src/core/meshcop/joiner.hpp b/src/core/meshcop/joiner.hpp index 0c729db98..93648b43f 100644 --- a/src/core/meshcop/joiner.hpp +++ b/src/core/meshcop/joiner.hpp @@ -68,10 +68,10 @@ public: * * @param[in] aPSKd A pointer to the PSKd. * @param[in] aProvisioningUrl A pointer to the Provisioning URL (may be NULL). - * @param[in] aVendorName A pointer to the Vendor Name (must be static). - * @param[in] aVendorModel A pointer to the Vendor Model (must be static). - * @param[in] aVendorSwVersion A pointer to the Vendor SW Version (must be static). - * @param[in] aVendorData A pointer to the Vendor Data (must be static). + * @param[in] aVendorName A pointer to the Vendor Name (may be NULL). + * @param[in] aVendorModel A pointer to the Vendor Model (may be NULL). + * @param[in] aVendorSwVersion A pointer to the Vendor SW Version (may be NULL). + * @param[in] aVendorData A pointer to the Vendor Data (may be NULL). * @param[in] aCallback A pointer to a function that is called when the join operation completes. * @param[in] aContext A pointer to application-specific context. * @@ -152,6 +152,13 @@ private: otError Connect(JoinerRouter &aRouter); void Finish(otError aError); uint8_t CalculatePriority(int8_t aRssi, bool aSteeringDataAllowsAny); + + otError PrepareJoinerFinalizeMessage(const char *aProvisioningUrl, + const char *aVendorName, + const char *aVendorModel, + const char *aVendorSwVersion, + const char *aVendorData); + void FreeJoinerFinalizeMessage(void); void SendJoinerFinalize(void); void SendJoinerEntrustResponse(const Coap::Message &aRequest, const Ip6::MessageInfo &aRequestInfo); @@ -167,10 +174,7 @@ private: JoinerRouter mJoinerRouters[OPENTHREAD_CONFIG_MAX_JOINER_ROUTER_ENTRIES]; uint16_t mJoinerRouterIndex; - const char *mVendorName; - const char *mVendorModel; - const char *mVendorSwVersion; - const char *mVendorData; + Coap::Message *mFinalizeMessage; TimerMilli mTimer; Coap::Resource mJoinerEntrust;