From 0269ca3ec540f52b77db640794119eae2743cd94 Mon Sep 17 00:00:00 2001 From: "jihwan.seo" Date: Mon, 14 Sep 2015 21:29:03 +0900 Subject: [PATCH] Clean up the stack code of csdk to check NULL Change-Id: I71cbfc835d1710a472d3089c2686e4e980a05ce4 Signed-off-by: jihwan.seo Reviewed-on: https://gerrit.iotivity.org/gerrit/2511 Tested-by: jenkins-iotivity Reviewed-by: Patrick Lankswert --- resource/csdk/stack/src/occlientcb.c | 18 +++++++------- resource/csdk/stack/src/occollection.c | 17 +++++++++++--- resource/csdk/stack/src/ocobserve.c | 19 ++++++++------- resource/csdk/stack/src/ocpayload.c | 12 +++++++++- resource/csdk/stack/src/ocpayloadparse.c | 39 +++++++++++++++++++++++++++++-- resource/csdk/stack/src/ocresource.c | 20 +++++++--------- resource/csdk/stack/src/ocserverrequest.c | 36 +++++++++++++++++++--------- 7 files changed, 115 insertions(+), 46 deletions(-) diff --git a/resource/csdk/stack/src/occlientcb.c b/resource/csdk/stack/src/occlientcb.c index ecfcfb0..4329c39 100644 --- a/resource/csdk/stack/src/occlientcb.c +++ b/resource/csdk/stack/src/occlientcb.c @@ -55,12 +55,12 @@ AddClientCB (ClientCB** clientCB, OCCallbackData* cbData, ClientCB *cbNode = NULL; - #ifdef WITH_PRESENCE +#ifdef WITH_PRESENCE if(method == OC_REST_PRESENCE) { // Retrieve the presence callback structure for this specific requestUri. cbNode = GetClientCB(NULL, 0, NULL, requestUri); } - #endif // WITH_PRESENCE +#endif // WITH_PRESENCE if(!cbNode)// If it does not already exist, create new node. { @@ -124,7 +124,7 @@ AddClientCB (ClientCB** clientCB, OCCallbackData* cbData, *handle = cbNode->handle; } - #ifdef WITH_PRESENCE +#ifdef WITH_PRESENCE if(method == OC_REST_PRESENCE && resourceTypeName) { // Amend the found or created node by adding a new resourceType to it. @@ -135,14 +135,14 @@ AddClientCB (ClientCB** clientCB, OCCallbackData* cbData, { OICFree(resourceTypeName); } - #else +#else OICFree(resourceTypeName); - #endif +#endif return OC_STACK_OK; - exit: - return OC_STACK_NO_MEMORY; +exit: + return OC_STACK_NO_MEMORY; } void DeleteClientCB(ClientCB * cbNode) @@ -162,7 +162,7 @@ void DeleteClientCB(ClientCB * cbNode) cbNode->deleteCallback(cbNode->context); } - #ifdef WITH_PRESENCE +#ifdef WITH_PRESENCE if(cbNode->presence) { OICFree(cbNode->presence->timeOut); @@ -180,7 +180,7 @@ void DeleteClientCB(ClientCB * cbNode) pointer = next; } } - #endif // WITH_PRESENCE +#endif // WITH_PRESENCE OICFree(cbNode); cbNode = NULL; } diff --git a/resource/csdk/stack/src/occollection.c b/resource/csdk/stack/src/occollection.c index de00ebb..5684f6a 100644 --- a/resource/csdk/stack/src/occollection.c +++ b/resource/csdk/stack/src/occollection.c @@ -102,7 +102,9 @@ ValidateQuery (const char *query, OCResourceHandle resource, OC_LOG(INFO, TAG, "Entering ValidateQuery"); if (!query) + { return OC_STACK_ERROR; + } if(!ifParam || !rtParam) { @@ -161,6 +163,7 @@ ValidateQuery (const char *query, OCResourceHandle resource, } token = strtok_r (NULL, OC_QUERY_SEPARATOR, &endStr); } + if (numFields > NUM_FIELDS_IN_QUERY) { // current release supports one IF value, one RT value and no other params @@ -274,6 +277,11 @@ HandleLinkedListInterface(OCEntityHandlerRequest *ehRequest, static OCStackResult HandleBatchInterface(OCEntityHandlerRequest *ehRequest) { + if (!ehRequest) + { + return OC_STACK_INVALID_PARAM; + } + OCStackResult stackRet = OC_STACK_OK; OCEntityHandlerResult ehResult = OC_EH_ERROR; OCResource * collResource = (OCResource *) ehRequest->resource; @@ -286,7 +294,10 @@ HandleBatchInterface(OCEntityHandlerRequest *ehRequest) if(stackRet == OC_STACK_OK) { - OCRepPayloadSetUri(payload, collResource->uri); + if (collResource) + { + OCRepPayloadSetUri(payload, collResource->uri); + } } if(stackRet == OC_STACK_OK) @@ -302,7 +313,7 @@ HandleBatchInterface(OCEntityHandlerRequest *ehRequest) if (stackRet == OC_STACK_OK) { - for (int i = 0; i < MAX_CONTAINED_RESOURCES; i++) + for (uint8_t i = 0; i < MAX_CONTAINED_RESOURCES; i++) { OCResource* temp = collResource->rsrcResources[i]; if (temp) @@ -344,7 +355,7 @@ uint8_t GetNumOfResourcesInCollection (OCResource *resource) if(resource) { uint8_t num = 0; - for (int i = 0; i < MAX_CONTAINED_RESOURCES; i++) + for (uint8_t i = 0; i < MAX_CONTAINED_RESOURCES; i++) { if (resource->rsrcResources[i]) { diff --git a/resource/csdk/stack/src/ocobserve.c b/resource/csdk/stack/src/ocobserve.c index 72f1b96..bbd3370 100644 --- a/resource/csdk/stack/src/ocobserve.c +++ b/resource/csdk/stack/src/ocobserve.c @@ -72,14 +72,14 @@ static OCQualityOfService DetermineObserverQoS(OCMethod method, { OC_LOG_V(INFO, TAG, "Current NON count for this observer is %d", resourceObserver->lowQosCount); - #ifdef WITH_PRESENCE +#ifdef WITH_PRESENCE if((resourceObserver->forceHighQos \ || resourceObserver->lowQosCount >= MAX_OBSERVER_NON_COUNT) \ && method != OC_REST_PRESENCE) - #else +#else if(resourceObserver->forceHighQos \ || resourceObserver->lowQosCount >= MAX_OBSERVER_NON_COUNT) - #endif +#endif { resourceObserver->lowQosCount = 0; // at some point we have to to send CON to check on the @@ -123,10 +123,10 @@ OCStackResult SendAllObserverNotification (OCMethod method, OCResource *resPtr, if (resourceObserver->resource == resPtr) { numObs++; - #ifdef WITH_PRESENCE +#ifdef WITH_PRESENCE if(method != OC_REST_PRESENCE) { - #endif +#endif qos = DetermineObserverQoS(method, resourceObserver, qos); result = AddServerRequest(&request, 0, 0, 1, OC_REST_GET, @@ -166,7 +166,7 @@ OCStackResult SendAllObserverNotification (OCMethod method, OCResource *resPtr, OCPayloadDestroy(ehRequest.payload); } } - #ifdef WITH_PRESENCE +#ifdef WITH_PRESENCE } else { @@ -207,7 +207,7 @@ OCStackResult SendAllObserverNotification (OCMethod method, OCResource *resPtr, OCPresencePayloadDestroy(presenceResBuf); } } - #endif +#endif // Since we are in a loop, set an error flag to indicate at least one error occurred. if (result != OC_STACK_OK) @@ -521,7 +521,7 @@ CreateObserveHeaderOption (CAHeaderOption_t **caHdrOpt, uint8_t numOptions, uint8_t observeFlag) { - if(!caHdrOpt) + if(!caHdrOpt || !ocHdrOpt) { return OC_STACK_INVALID_PARAM; } @@ -563,13 +563,14 @@ GetObserveHeaderOption (uint32_t * observationOption, { return OC_STACK_INVALID_PARAM; } - *observationOption = OC_OBSERVE_NO_OPTION; if(!options || !numOptions) { return OC_STACK_INVALID_PARAM; } + *observationOption = OC_OBSERVE_NO_OPTION; + for(uint8_t i = 0; i < *numOptions; i++) { if(options[i].protocolID == CA_COAP_ID && diff --git a/resource/csdk/stack/src/ocpayload.c b/resource/csdk/stack/src/ocpayload.c index ba2e408..6be1ffe 100755 --- a/resource/csdk/stack/src/ocpayload.c +++ b/resource/csdk/stack/src/ocpayload.c @@ -118,6 +118,11 @@ static OCRepPayloadValue* OCRepPayloadFindValue(const OCRepPayload* payload, con static void OCCopyPropertyValueArray(OCRepPayloadValue* dest, OCRepPayloadValue* source) { + if(!dest || !source) + { + return; + } + size_t dimTotal = calcDimTotal(source->arr.dimensions); switch(source->arr.type) { @@ -1223,7 +1228,7 @@ static OCResourcePayload* OCCopyResource(const OCResource* res, uint16_t port) OCStringLL* cur = pl->interfaces; ifPtr = ifPtr->next; - while(ifPtr) + while(ifPtr && cur) { cur->next = (OCStringLL*)OICCalloc(1, sizeof(OCStringLL)); if(!cur->next) @@ -1369,6 +1374,11 @@ void OCDevicePayloadDestroy(OCDevicePayload* payload) static void OCCopyPlatformInfo(const OCPlatformInfo* platformInfo, OCPlatformPayload* target) { + if(!platformInfo || !target) + { + return; + } + target->info.platformID = OICStrdup(platformInfo->platformID); target->info.manufacturerName = OICStrdup(platformInfo->manufacturerName); target->info.manufacturerUrl = OICStrdup(platformInfo->manufacturerUrl); diff --git a/resource/csdk/stack/src/ocpayloadparse.c b/resource/csdk/stack/src/ocpayloadparse.c index 81dede5..e3596da 100644 --- a/resource/csdk/stack/src/ocpayloadparse.c +++ b/resource/csdk/stack/src/ocpayloadparse.c @@ -113,6 +113,11 @@ void OCFreeOCStringLL(OCStringLL* ll); static OCStackResult OCParseSecurityPayload(OCPayload** outPayload, CborValue* arrayVal) { + if (!outPayload) + { + return OC_STACK_INVALID_PARAM; + } + bool err = false; char * securityData = NULL; @@ -151,6 +156,11 @@ static OCStackResult OCParseSecurityPayload(OCPayload** outPayload, CborValue* a static OCStackResult OCParseDiscoveryPayload(OCPayload** outPayload, CborValue* arrayVal) { + if (!outPayload) + { + return OC_STACK_INVALID_PARAM; + } + bool err = false; OCDiscoveryPayload* out = OCDiscoveryPayloadCreate(); @@ -356,6 +366,11 @@ static OCStackResult OCParseDiscoveryPayload(OCPayload** outPayload, CborValue* static OCStackResult OCParseDevicePayload(OCPayload** outPayload, CborValue* arrayVal) { + if (!outPayload) + { + return OC_STACK_INVALID_PARAM; + } + bool err = false; if(cbor_value_is_map(arrayVal)) @@ -427,6 +442,11 @@ static OCStackResult OCParseDevicePayload(OCPayload** outPayload, CborValue* arr static OCStackResult OCParsePlatformPayload(OCPayload** outPayload, CborValue* arrayVal) { + if (!outPayload) + { + return OC_STACK_INVALID_PARAM; + } + bool err = false; if(cbor_value_is_map(arrayVal)) @@ -861,6 +881,11 @@ static bool OCParseArray(OCRepPayload* out, const char* name, CborValue* contain static bool OCParseSingleRepPayload(OCRepPayload** outPayload, CborValue* repParent) { + if (!outPayload) + { + return false; + } + *outPayload = OCRepPayloadCreate(); OCRepPayload* curPayload = *outPayload; bool err = false; @@ -929,9 +954,9 @@ static bool OCParseSingleRepPayload(OCRepPayload** outPayload, CborValue* repPar while(!err && cbor_value_is_valid(&repMap)) { char* name; - err = err || cbor_value_dup_text_string(&repMap, &name, &len, NULL); + err = err || cbor_value_dup_text_string(&repMap, &name, &len, NULL); - err = err || cbor_value_advance(&repMap); + err = err || cbor_value_advance(&repMap); int64_t intval = 0; bool boolval = false; @@ -1003,6 +1028,11 @@ static bool OCParseSingleRepPayload(OCRepPayload** outPayload, CborValue* repPar } static OCStackResult OCParseRepPayload(OCPayload** outPayload, CborValue* arrayVal) { + if (!outPayload) + { + return OC_STACK_INVALID_PARAM; + } + bool err = false; OCRepPayload* rootPayload = NULL; @@ -1040,6 +1070,11 @@ static OCStackResult OCParseRepPayload(OCPayload** outPayload, CborValue* arrayV static OCStackResult OCParsePresencePayload(OCPayload** outPayload, CborValue* arrayVal) { + if (!outPayload) + { + return OC_STACK_INVALID_PARAM; + } + bool err = false; if(cbor_value_is_map(arrayVal)) { diff --git a/resource/csdk/stack/src/ocresource.c b/resource/csdk/stack/src/ocresource.c index b5da597..f60c08b 100644 --- a/resource/csdk/stack/src/ocresource.c +++ b/resource/csdk/stack/src/ocresource.c @@ -340,8 +340,7 @@ OCStackResult DetermineResourceHandling (const OCServerRequest *request, } else { - OCResource *resourcePtr = NULL; - resourcePtr = FindResourceByUri((const char*)request->resourceUrl); + OCResource *resourcePtr = FindResourceByUri((const char*)request->resourceUrl); *resource = resourcePtr; if (!resourcePtr) { @@ -493,7 +492,7 @@ static bool includeThisResourceInResponse(OCResource *resource, return false; } - if ( resource->resourceProperties & OC_EXPLICIT_DISCOVERABLE) + if (resource->resourceProperties & OC_EXPLICIT_DISCOVERABLE) { /* * At least one valid filter should be available to @@ -507,7 +506,7 @@ static bool includeThisResourceInResponse(OCResource *resource, return false; } } - else if ( !(resource->resourceProperties & OC_ACTIVE) || + else if (!(resource->resourceProperties & OC_ACTIVE) || !(resource->resourceProperties & OC_DISCOVERABLE)) { OC_LOG_V(INFO, TAG, "%s not ACTIVE or DISCOVERABLE", resource->uri); @@ -640,7 +639,7 @@ static OCStackResult HandleVirtualResource (OCServerRequest *request, OCResource * request is unicast, it should send an error(RESOURCE_NOT_FOUND - 404) response. */ - #ifdef WITH_PRESENCE +#ifdef WITH_PRESENCE if ((virtualUriInRequest == OC_PRESENCE) && (resource->resourceProperties & OC_ACTIVE)) { @@ -648,7 +647,7 @@ static OCStackResult HandleVirtualResource (OCServerRequest *request, OCResource SendPresenceNotification(resource->rsrcType, OC_PRESENCE_TRIGGER_CHANGE); } else - #endif +#endif { if(discoveryResult == OC_STACK_OK) { @@ -1008,7 +1007,7 @@ OCStackResult SavePlatformInfo(OCPlatformInfo info) } else { - OC_LOG(ERROR, TAG, "Platform info saved."); + OC_LOG(INFO, TAG, "Platform info saved."); } return res; @@ -1055,8 +1054,7 @@ OCStackResult SaveDeviceInfo(OCDeviceInfo info) OC_LOG(INFO, TAG, "Device initialized successfully."); return OC_STACK_OK; - exit: - DeleteDeviceInfo(); - return res; - +exit: + DeleteDeviceInfo(); + return res; } diff --git a/resource/csdk/stack/src/ocserverrequest.c b/resource/csdk/stack/src/ocserverrequest.c index 4122a33..92c0593 100644 --- a/resource/csdk/stack/src/ocserverrequest.c +++ b/resource/csdk/stack/src/ocserverrequest.c @@ -56,6 +56,11 @@ static struct OCServerResponse * serverResponseList = NULL; */ static OCStackResult AddServerResponse (OCServerResponse ** response, OCRequestHandle requestHandle) { + if (!response) + { + return OC_STACK_INVALID_PARAM; + } + OCServerResponse * serverResponse = NULL; serverResponse = (OCServerResponse *) OICCalloc(1, sizeof(OCServerResponse)); @@ -245,6 +250,11 @@ OCStackResult AddServerRequest (OCServerRequest ** request, uint16_t coapID, char * resourceUrl, size_t reqTotalSize, OCPayloadFormat acceptFormat, const OCDevAddr *devAddr) { + if (!request) + { + return OC_STACK_INVALID_PARAM; + } + OCServerRequest * serverRequest = NULL; serverRequest = (OCServerRequest *) OICCalloc(1, sizeof(OCServerRequest) + @@ -390,7 +400,7 @@ void FindAndDeleteServerRequest(OCServerRequest * serverRequest) CAResponseResult_t ConvertEHResultToCAResult (OCEntityHandlerResult result, OCMethod method) { - CAResponseResult_t caResult = CA_BAD_REQ; + CAResponseResult_t caResult; switch (result) { @@ -491,6 +501,11 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) { responseInfo.info.type = CA_MSG_NONCONFIRM; } + else + { + OC_LOG(ERROR, TAG, "default responseInfo type is NON"); + responseInfo.info.type = CA_MSG_NONCONFIRM; + } char rspToken[CA_MAX_TOKEN_LEN + 1] = {}; responseInfo.info.messageId = serverRequest->coapID; @@ -531,8 +546,8 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) responseInfo.info.options[0].optionLength = sizeof(uint32_t); uint8_t* observationData = (uint8_t*)responseInfo.info.options[0].optionData; uint32_t observationOption= serverRequest->observationOption; - size_t i; - for (i=sizeof(uint32_t); i; --i) + + for (size_t i=sizeof(uint32_t); i; --i) { observationData[i-1] = observationOption & 0xFF; observationOption >>=8; @@ -606,7 +621,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) #endif }; - int size = sizeof(CAConnTypes)/ sizeof(CATransportAdapter_t); + size_t size = sizeof(CAConnTypes)/ sizeof(CATransportAdapter_t); CATransportAdapter_t adapter = responseEndpoint.adapter; // Default adapter, try to send response out on all adapters. @@ -631,7 +646,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) result = OC_STACK_OK; OCStackResult tempResult = OC_STACK_OK; - for(int i = 0; i < size; i++ ) + for(size_t i = 0; i < size; i++ ) { responseEndpoint.adapter = (CATransportAdapter_t)(adapter & CAConnTypes[i]); if(responseEndpoint.adapter) @@ -677,10 +692,6 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) */ OCStackResult HandleAggregateResponse(OCEntityHandlerResponse * ehResponse) { - OCStackResult stackRet = OC_STACK_ERROR; - OCServerRequest * serverRequest = NULL; - OCServerResponse * serverResponse = NULL; - if(!ehResponse || !ehResponse->payload) { OC_LOG(ERROR, TAG, "HandleAggregateResponse invalid parameters"); @@ -689,9 +700,12 @@ OCStackResult HandleAggregateResponse(OCEntityHandlerResponse * ehResponse) OC_LOG(INFO, TAG, "Inside HandleAggregateResponse"); - serverRequest = GetServerRequestUsingHandle((OCServerRequest *)ehResponse->requestHandle); - serverResponse = GetServerResponseUsingHandle((OCServerRequest *)ehResponse->requestHandle); + OCServerRequest *serverRequest = GetServerRequestUsingHandle((OCServerRequest *) + ehResponse->requestHandle); + OCServerResponse *serverResponse = GetServerResponseUsingHandle((OCServerRequest *) + ehResponse->requestHandle); + OCStackResult stackRet = OC_STACK_ERROR; if(serverRequest) { if(!serverResponse) -- 2.7.4