[IOT-1880] Adding Fixes for issues generated from static analyzer tool in Notificatio...
authorPoovizhi <poovizhi.a@samsung.com>
Fri, 3 Mar 2017 10:24:09 +0000 (15:54 +0530)
committerUze Choi <uzchoi@samsung.com>
Mon, 20 Mar 2017 09:11:16 +0000 (09:11 +0000)
1) In JNINotificationConsumer.cpp, Getting native Provider object is common code for all the native methods.
   Added a separate method ' getNativeProvider' to do this, so that method size and complexity will be reduced.

2) JNiNotificationConsumer.cpp Line 1379, and NSTopicslist.cpp Line 59 has changes to fix the issue of Unreachable code.

3) Copy constructor and copy assignment operator are added  in class 'NSAcceptedProviders' & 'NSAcceptedConsumers'  which has dynamically allocated data members

4) In NSAcceptedProviders class, getProviders() method is changed to const since it is being used by the copy constructors and hence
   modified the member variable 'm_mutex' to be mutable.

5) In NSAcceptedConsumers class, getConsumers() method is changed to const since it is being used by the copy constructors
   and hence modified the member variable 'm_mutex' to be mutable.

6) In  NotiListener.java, the NULL check for mProviderSample is moved above the first instance where mProviderSample is beig used.

Change-Id: Ic18c3d9797a02a73f5397192b21e7dda5926119e
Signed-off-by: Poovizhi <poovizhi.a@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/17653
Reviewed-by: Uze Choi <uzchoi@samsung.com>
Tested-by: Uze Choi <uzchoi@samsung.com>
service/notification/android/notification-service/src/main/jni/consumer/JniNotificationConsumer.cpp [changed mode: 0755->0644]
service/notification/cpp-wrapper/common/NSTopicsList.cpp [changed mode: 0755->0644]
service/notification/cpp-wrapper/consumer/inc/NSAcceptedProviders.h [changed mode: 0755->0644]
service/notification/cpp-wrapper/consumer/src/NSAcceptedProviders.cpp [changed mode: 0755->0644]
service/notification/cpp-wrapper/consumer/src/NSConsumerService.cpp [changed mode: 0755->0644]
service/notification/cpp-wrapper/provider/inc/NSAcceptedConsumers.h [changed mode: 0755->0644]
service/notification/cpp-wrapper/provider/src/NSAcceptedConsumers.cpp [changed mode: 0755->0644]
service/notification/examples/android/NotiProviderExample/app/src/main/java/org/iotivity/service/ns/sample/provider/NotiListener.java

old mode 100755 (executable)
new mode 100644 (file)
index 7abe708..a9eb888
@@ -71,6 +71,24 @@ static JNIEnv *GetJNIEnv(jint *ret)
     }\r
 }\r
 \r
+static jlong getNativeProvider(JNIEnv *env, jobject jObj)\r
+{\r
+\r
+    jclass providerClass = env->GetObjectClass(jObj);\r
+    if (!providerClass)\r
+    {\r
+        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
+        return 0;\r
+    }\r
+    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
+    if (!nativeHandle)\r
+    {\r
+        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
+        return 0;\r
+    }\r
+    return (env->GetLongField(jObj, nativeHandle));\r
+}\r
+\r
 jobject getJavaProviderState(JNIEnv *env, OIC::Service::NSProviderState state)\r
 {\r
     NS_LOGD ("ConsumerService_getJavaProviderState - IN");\r
@@ -297,7 +315,7 @@ const char *getNativeTopicName(JNIEnv *env,  jobject jTopic)
     }\r
     else\r
     {\r
-        NS_LOGI (TAG, "Info: topicName is null");\r
+        NS_LOGI ("topicName is null");\r
     }\r
     NS_LOGD ("ConsumerService_getNativeTopicName - OUT");\r
     return topicName;\r
@@ -394,6 +412,7 @@ jobject getJavaProvider(JNIEnv *env, std::shared_ptr<OIC::Service::NSProvider> p
     if (!cls_provider)\r
     {\r
         NS_LOGE ("Failed to Get ObjectClass for Provider");\r
+        delete objectHolder;\r
         return NULL;\r
     }\r
     jmethodID mid_provider = env->GetMethodID(\r
@@ -401,12 +420,14 @@ jobject getJavaProvider(JNIEnv *env, std::shared_ptr<OIC::Service::NSProvider> p
     if (!mid_provider)\r
     {\r
         NS_LOGE ("Failed to Get MethodID for Provider<init>");\r
+        delete objectHolder;\r
         return NULL;\r
     }\r
     jobject obj_provider = env->NewObject(cls_provider, mid_provider, jProviderId);\r
     if (!obj_provider)\r
     {\r
         NS_LOGE ("Failed to create new Object for Provider");\r
+        delete objectHolder;\r
         return NULL;\r
     }\r
 \r
@@ -414,6 +435,7 @@ jobject getJavaProvider(JNIEnv *env, std::shared_ptr<OIC::Service::NSProvider> p
     if (!nativeHandle)\r
     {\r
         NS_LOGE ("Failed to get nativeHandle for Provider");\r
+        delete objectHolder;\r
         return NULL;\r
     }\r
     env->SetLongField(obj_provider, nativeHandle, pProvider);\r
@@ -1066,20 +1088,8 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeSubs
 {\r
     NS_LOGD ("Provider_Subscribe -IN");\r
     OIC::Service::NSResult result  = OIC::Service::NSResult::ERROR;\r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return ;\r
-    }\r
 \r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return ;\r
-    }\r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     if (jProvider)\r
     {\r
         NS_LOGD ("calling subscribe on mNativeHandle");\r
@@ -1114,20 +1124,7 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeUnsu
 {\r
     NS_LOGD ("Provider_UnSubscribe -IN");\r
     OIC::Service::NSResult result  = OIC::Service::NSResult::ERROR;\r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return ;\r
-    }\r
-\r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return ;\r
-    }\r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     if (jProvider)\r
     {\r
         NS_LOGD ("calling subscribe on mNativeHandle");\r
@@ -1168,25 +1165,12 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeSend
         return ;\r
     }\r
 \r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return ;\r
-    }\r
-\r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return ;\r
-    }\r
     uint64_t messageId = (uint64_t) jMessageId;\r
 \r
     NS_LOGD ("!!!!!!jMessageId: %lld", jMessageId);\r
     NS_LOGD ("!!!!!!messageId: %lld", messageId);\r
 \r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     if (jProvider)\r
     {\r
         NS_LOGD ("calling SendSyncInfo on mNativeHandle");\r
@@ -1228,20 +1212,7 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeSetL
         return ;\r
     }\r
 \r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return ;\r
-    }\r
-\r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return ;\r
-    }\r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     if (jProvider)\r
     {\r
         NS_LOGD ("calling SetListener on mNativeHandle");\r
@@ -1278,20 +1249,7 @@ JNIEXPORT jobject JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeG
 (JNIEnv *env, jobject jObj)\r
 {\r
     NS_LOGD ("Provider_nativeGetTopicList - IN");\r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return NULL;\r
-    }\r
-\r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return NULL;\r
-    }\r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     std::shared_ptr<OIC::Service::NSTopicsList> topicList = nullptr;\r
     if (jProvider)\r
     {\r
@@ -1341,20 +1299,7 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeUpda
         return;\r
     }\r
 \r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return;\r
-    }\r
-\r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return;\r
-    }\r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     OIC::Service::NSResult result = OIC::Service::NSResult::ERROR;\r
     if (jProvider)\r
     {\r
@@ -1389,20 +1334,7 @@ JNIEXPORT jobject JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeG
 (JNIEnv *env, jobject jObj)\r
 {\r
     NS_LOGD ("Provider_nativeGetProviderState - IN");\r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return NULL;\r
-    }\r
-\r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return NULL;\r
-    }\r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     OIC::Service::NSProviderState state = OIC::Service::NSProviderState::DENY;\r
     if (jProvider)\r
     {\r
@@ -1434,20 +1366,9 @@ JNIEXPORT jboolean JNICALL Java_org_iotivity_service_ns_consumer_Provider_native
 (JNIEnv *env, jobject jObj)\r
 {\r
     NS_LOGD ("nativeIsSubscribed - IN");\r
-    jclass providerClass = env->GetObjectClass(jObj);\r
-    if (!providerClass)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");\r
-        return (jboolean)false;\r
-    }\r
+    jboolean subscribedState = false;\r
 \r
-    jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");\r
-    if (!nativeHandle)\r
-    {\r
-        ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");\r
-        return (jboolean)false;\r
-    }\r
-    jlong jProvider = env->GetLongField(jObj, nativeHandle);\r
+    jlong jProvider = getNativeProvider(env, jObj);\r
     if (jProvider)\r
     {\r
         NS_LOGD ("calling isSubscribe on mNativeHandle");\r
@@ -1455,13 +1376,14 @@ JNIEXPORT jboolean JNICALL Java_org_iotivity_service_ns_consumer_Provider_native
             reinterpret_cast<JniSharedObjectHolder<OIC::Service::NSProvider> *>(jProvider);\r
         try\r
         {\r
-            return (jboolean) objectHolder->get()->isSubscribed();\r
+            subscribedState = (jboolean)objectHolder->get()->isSubscribed();\r
         }\r
         catch (OIC::Service::NSException ex)\r
         {\r
             ThrowNSException(NATIVE_EXCEPTION, ex.what());\r
             return (jboolean)false;\r
         }\r
+        return subscribedState;\r
     }\r
     else\r
     {\r
old mode 100755 (executable)
new mode 100644 (file)
index c9a6970..6235833
@@ -35,7 +35,7 @@ namespace OIC
             while (topicsNode != nullptr)
             {
                 m_topicsList.push_back(new NSTopic(
-                    topicsNode->topicName, (NSTopic::NSTopicState)topicsNode->state));
+                                           topicsNode->topicName, (NSTopic::NSTopicState)topicsNode->state));
                 topicsNode = topicsNode->next;
             }
 
@@ -55,8 +55,8 @@ namespace OIC
             {
                 this->m_topicsList.push_back(new NSTopic(it.getTopicName(), it.getState()));
             }
-            return *this;
             m_modifiable = false;
+            return *this;
         }
 
         NSTopicsList::~NSTopicsList()
@@ -70,7 +70,7 @@ namespace OIC
 
         void NSTopicsList::addTopic(const std::string &topicName, NSTopic::NSTopicState state)
         {
-            if(m_modifiable)
+            if (m_modifiable)
             {
                 m_topicsList.push_back(new NSTopic(topicName, state));
             }
@@ -82,7 +82,7 @@ namespace OIC
 
         void NSTopicsList::removeTopic(const std::string &topicName)
         {
-            if(m_modifiable)
+            if (m_modifiable)
             {
                 for (auto it : m_topicsList)
                 {
@@ -108,7 +108,7 @@ namespace OIC
             }
             return topicList;
         }
-       
+
         //Below method restricts the application from illegally modifying Topics when
         //Provider is in Invalid state. By calling the API, the service prevents and protects
         //the integrity of TopicsList updation when the associated object is Invalid
old mode 100755 (executable)
new mode 100644 (file)
index 3922661..0d79de8
@@ -55,6 +55,19 @@ namespace OIC
                 }
 
                 /**
+                    * Copy Constructor of NSAcceptedProviders.
+                    *
+                    */
+                NSAcceptedProviders(const NSAcceptedProviders &);
+
+                /**
+                    * Copy assignment operator of NSAcceptedProviders.
+                    *
+                    * @return NSAcceptedProviders object reference
+                    */
+                NSAcceptedProviders &operator=(const NSAcceptedProviders &);
+
+                /**
                       * Destructor of NSAcceptedProviders.
                       */
                 ~NSAcceptedProviders()
@@ -107,11 +120,11 @@ namespace OIC
                       *  get the map of providers accepted.
                       * @return m_providers  -map of accepted providers
                       */
-                std::map<std::string, std::shared_ptr<NSProvider> > getProviders();
+                std::map<std::string, std::shared_ptr<NSProvider> > getProviders() const;
 
             private :
                 std::map<std::string, std::shared_ptr<NSProvider> > m_providers;
-                std::mutex m_mutex;
+                mutable std::mutex m_mutex;
         };
     }
 }
old mode 100755 (executable)
new mode 100644 (file)
index ac2408c..1e3edf5
@@ -27,6 +27,19 @@ namespace OIC
 {
     namespace Service
     {
+        NSAcceptedProviders::NSAcceptedProviders(const NSAcceptedProviders &providers)
+        {
+            removeProviders();
+            m_providers.insert((providers.getProviders()).begin(), (providers.getProviders()).end());
+        }
+
+        NSAcceptedProviders &NSAcceptedProviders::operator=(const NSAcceptedProviders &providers)
+        {
+            removeProviders();
+            this->m_providers.insert((providers.getProviders()).begin(), (providers.getProviders()).end());
+            return *this;
+        }
+
         std::shared_ptr<NSProvider> NSAcceptedProviders::getProvider(const std::string &id)
         {
             std::lock_guard<std::mutex> lock(m_mutex);
@@ -87,7 +100,7 @@ namespace OIC
             return;
         }
 
-        std::map<std::string, std::shared_ptr<NSProvider> > NSAcceptedProviders::getProviders()
+        std::map<std::string, std::shared_ptr<NSProvider> > NSAcceptedProviders::getProviders() const
         {
             std::lock_guard<std::mutex> lock(m_mutex);
             return m_providers;
old mode 100755 (executable)
new mode 100644 (file)
index d242919..fdfac60
@@ -147,7 +147,10 @@ namespace OIC
                 else if (state == NS_STOPPED)
                 {
                     oldProvider->setProviderSubscribedState(NSProviderSubscribedState::DENY);
-                    oldProvider->getTopicList()->unsetModifiability();
+                    if (NULL != oldProvider->getTopicList())
+                    {
+                        oldProvider->getTopicList()->unsetModifiability();
+                    }
                     NSConsumerService::getInstance()->getAcceptedProviders()->removeProvider(
                         oldProvider->getProviderId());
                     NS_LOG(DEBUG, "initiating the State callback : NS_STOPPED");
old mode 100755 (executable)
new mode 100644 (file)
index a57654f..b4ed05d
@@ -54,6 +54,19 @@ namespace OIC
                 }
 
                 /**
+                    * Copy Constructor of NSAcceptedConsumers.
+                    *
+                    */
+                NSAcceptedConsumers(const NSAcceptedConsumers &);
+
+                /**
+                    * Copy assignment operator of NSAcceptedConsumers.
+                    *
+                    * @return NSAcceptedConsumers object reference
+                    */
+                NSAcceptedConsumers &operator=(const NSAcceptedConsumers &);
+
+                /**
                       * Destructor of NSAcceptedConsumers.
                       */
                 ~NSAcceptedConsumers()
@@ -106,11 +119,11 @@ namespace OIC
                       *  get the map of Consumers accepted.
                       * @return m_consumers  -map of accepted Consumers
                       */
-                std::map<std::string, std::shared_ptr<NSConsumer> > getConsumers();
+                std::map<std::string, std::shared_ptr<NSConsumer> > getConsumers() const;
 
             private :
                 std::map<std::string, std::shared_ptr<NSConsumer> > m_consumers;
-                std::mutex m_mutex;
+                mutable std::mutex m_mutex;
         };
     }
 }
old mode 100755 (executable)
new mode 100644 (file)
index 241317a..8eda25d
@@ -29,6 +29,19 @@ namespace OIC
 {
     namespace Service
     {
+        NSAcceptedConsumers::NSAcceptedConsumers(const NSAcceptedConsumers &consumers)
+        {
+            removeConsumers();
+            m_consumers.insert((consumers.getConsumers()).begin(), (consumers.getConsumers()).end());
+        }
+
+        NSAcceptedConsumers &NSAcceptedConsumers::operator=(const NSAcceptedConsumers &consumers)
+        {
+            removeConsumers();
+            this->m_consumers.insert((consumers.getConsumers()).begin(), (consumers.getConsumers()).end());
+            return *this;
+        }
+
         std::shared_ptr<NSConsumer> NSAcceptedConsumers::getConsumer(const std::string &id)
         {
             std::lock_guard<std::mutex> lock(m_mutex);
@@ -89,7 +102,7 @@ namespace OIC
             return;
         }
 
-        std::map<std::string, std::shared_ptr<NSConsumer> > NSAcceptedConsumers::getConsumers()
+        std::map<std::string, std::shared_ptr<NSConsumer> > NSAcceptedConsumers::getConsumers() const
         {
             std::lock_guard<std::mutex> lock(m_mutex);
             return m_consumers;
index a25e351..02e2395 100755 (executable)
@@ -110,6 +110,10 @@ public class NotiListener extends NotificationListenerService {
         Log.i(TAG, "Title : " + title);
         Log.i(TAG, "Body : " + body);
 
+        if(mProviderSample == null) {
+            Log.e(TAG, "mProviderSample NULL");
+            return;
+        }
         Message notiMessage = mProviderSample.createMessage();
         if(notiMessage == null)
         {
@@ -126,11 +130,7 @@ public class NotiListener extends NotificationListenerService {
         notiMessage.setTime("12:10");
         MediaContents media = new MediaContents("daasd");
         notiMessage.setMediaContents(media);
-        if (mProviderSample != null) {
-            mProviderSample.sendMessage(notiMessage);
-        } else {
-            Log.i(TAG, "providerExample is NULL");
-        }
+        mProviderSample.sendMessage(notiMessage);
     }
 
     @Override