Commit 47aca923 authored by Poovizhi's avatar Poovizhi Committed by Uze Choi

[IOT-1880] Adding Fixes for issues generated from static analyzer tool in...

[IOT-1880] Adding Fixes for issues generated from static analyzer tool in Notification service c++ , JNI layer

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: default avatarPoovizhi <poovizhi.a@samsung.com>
Reviewed-on: https://gerrit.iotivity.org/gerrit/17653Reviewed-by: default avatarUze Choi <uzchoi@samsung.com>
Tested-by: default avatarUze Choi <uzchoi@samsung.com>
parent 61fb2859
......@@ -71,6 +71,24 @@ static JNIEnv *GetJNIEnv(jint *ret)
}
}
static jlong getNativeProvider(JNIEnv *env, jobject jObj)
{
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return 0;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return 0;
}
return (env->GetLongField(jObj, nativeHandle));
}
jobject getJavaProviderState(JNIEnv *env, OIC::Service::NSProviderState state)
{
NS_LOGD ("ConsumerService_getJavaProviderState - IN");
......@@ -297,7 +315,7 @@ const char *getNativeTopicName(JNIEnv *env, jobject jTopic)
}
else
{
NS_LOGI (TAG, "Info: topicName is null");
NS_LOGI ("topicName is null");
}
NS_LOGD ("ConsumerService_getNativeTopicName - OUT");
return topicName;
......@@ -394,6 +412,7 @@ jobject getJavaProvider(JNIEnv *env, std::shared_ptr<OIC::Service::NSProvider> p
if (!cls_provider)
{
NS_LOGE ("Failed to Get ObjectClass for Provider");
delete objectHolder;
return NULL;
}
jmethodID mid_provider = env->GetMethodID(
......@@ -401,12 +420,14 @@ jobject getJavaProvider(JNIEnv *env, std::shared_ptr<OIC::Service::NSProvider> p
if (!mid_provider)
{
NS_LOGE ("Failed to Get MethodID for Provider<init>");
delete objectHolder;
return NULL;
}
jobject obj_provider = env->NewObject(cls_provider, mid_provider, jProviderId);
if (!obj_provider)
{
NS_LOGE ("Failed to create new Object for Provider");
delete objectHolder;
return NULL;
}
......@@ -414,6 +435,7 @@ jobject getJavaProvider(JNIEnv *env, std::shared_ptr<OIC::Service::NSProvider> p
if (!nativeHandle)
{
NS_LOGE ("Failed to get nativeHandle for Provider");
delete objectHolder;
return NULL;
}
env->SetLongField(obj_provider, nativeHandle, pProvider);
......@@ -1066,20 +1088,8 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeSubs
{
NS_LOGD ("Provider_Subscribe -IN");
OIC::Service::NSResult result = OIC::Service::NSResult::ERROR;
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return ;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return ;
}
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
if (jProvider)
{
NS_LOGD ("calling subscribe on mNativeHandle");
......@@ -1114,20 +1124,7 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeUnsu
{
NS_LOGD ("Provider_UnSubscribe -IN");
OIC::Service::NSResult result = OIC::Service::NSResult::ERROR;
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return ;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return ;
}
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
if (jProvider)
{
NS_LOGD ("calling subscribe on mNativeHandle");
......@@ -1168,25 +1165,12 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeSend
return ;
}
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return ;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return ;
}
uint64_t messageId = (uint64_t) jMessageId;
NS_LOGD ("!!!!!!jMessageId: %lld", jMessageId);
NS_LOGD ("!!!!!!messageId: %lld", messageId);
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
if (jProvider)
{
NS_LOGD ("calling SendSyncInfo on mNativeHandle");
......@@ -1228,20 +1212,7 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeSetL
return ;
}
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return ;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return ;
}
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
if (jProvider)
{
NS_LOGD ("calling SetListener on mNativeHandle");
......@@ -1278,20 +1249,7 @@ JNIEXPORT jobject JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeG
(JNIEnv *env, jobject jObj)
{
NS_LOGD ("Provider_nativeGetTopicList - IN");
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return NULL;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return NULL;
}
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
std::shared_ptr<OIC::Service::NSTopicsList> topicList = nullptr;
if (jProvider)
{
......@@ -1341,20 +1299,7 @@ JNIEXPORT void JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeUpda
return;
}
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return;
}
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
OIC::Service::NSResult result = OIC::Service::NSResult::ERROR;
if (jProvider)
{
......@@ -1389,20 +1334,7 @@ JNIEXPORT jobject JNICALL Java_org_iotivity_service_ns_consumer_Provider_nativeG
(JNIEnv *env, jobject jObj)
{
NS_LOGD ("Provider_nativeGetProviderState - IN");
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return NULL;
}
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return NULL;
}
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
OIC::Service::NSProviderState state = OIC::Service::NSProviderState::DENY;
if (jProvider)
{
......@@ -1434,20 +1366,9 @@ JNIEXPORT jboolean JNICALL Java_org_iotivity_service_ns_consumer_Provider_native
(JNIEnv *env, jobject jObj)
{
NS_LOGD ("nativeIsSubscribed - IN");
jclass providerClass = env->GetObjectClass(jObj);
if (!providerClass)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to Get ObjectClass for Provider");
return (jboolean)false;
}
jboolean subscribedState = false;
jfieldID nativeHandle = env->GetFieldID(providerClass, "mNativeHandle", "J");
if (!nativeHandle)
{
ThrowNSException(JNI_INVALID_VALUE, "Failed to get nativeHandle for Provider");
return (jboolean)false;
}
jlong jProvider = env->GetLongField(jObj, nativeHandle);
jlong jProvider = getNativeProvider(env, jObj);
if (jProvider)
{
NS_LOGD ("calling isSubscribe on mNativeHandle");
......@@ -1455,13 +1376,14 @@ JNIEXPORT jboolean JNICALL Java_org_iotivity_service_ns_consumer_Provider_native
reinterpret_cast<JniSharedObjectHolder<OIC::Service::NSProvider> *>(jProvider);
try
{
return (jboolean) objectHolder->get()->isSubscribed();
subscribedState = (jboolean)objectHolder->get()->isSubscribed();
}
catch (OIC::Service::NSException ex)
{
ThrowNSException(NATIVE_EXCEPTION, ex.what());
return (jboolean)false;
}
return subscribedState;
}
else
{
......
......@@ -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
......
......@@ -54,6 +54,19 @@ namespace OIC
removeProviders();
}
/**
* Copy Constructor of NSAcceptedProviders.
*
*/
NSAcceptedProviders(const NSAcceptedProviders &);
/**
* Copy assignment operator of NSAcceptedProviders.
*
* @return NSAcceptedProviders object reference
*/
NSAcceptedProviders &operator=(const NSAcceptedProviders &);
/**
* Destructor of 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;
};
}
}
......
......@@ -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;
......
......@@ -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");
......
......@@ -53,6 +53,19 @@ namespace OIC
removeConsumers();
}
/**
* Copy Constructor of NSAcceptedConsumers.
*
*/
NSAcceptedConsumers(const NSAcceptedConsumers &);
/**
* Copy assignment operator of NSAcceptedConsumers.
*
* @return NSAcceptedConsumers object reference
*/
NSAcceptedConsumers &operator=(const NSAcceptedConsumers &);
/**
* Destructor of 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;
};
}
}
......
......@@ -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;
......
......@@ -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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment