OCF Bridging #588

Danielius1922 · 2024-01-11T12:36:19Z

No description provided.

- bugfix: oc_ri_delete_app_resources_per_device() => fix infinite loop - remove function: oc_resource_set_data_remover() => not necessary any more

…e = false oc_virtual_device_t : remove ecosystem_device field

…ce too

merge bridging feature

…clean up codes

Merge bridging

ocf-conformance-test-tool · 2024-01-11T12:36:37Z

🎉 Thank you for your code contribution! To guarantee the change/addition is conformant to the OCF Specification, we would like to ask you to execute OCF Conformance Testing of your change ☝️ when your work is ready to be reviewed.

ℹ️ To verify your latest change (bcb3671), label this PR with OCF Conformance Testing.

⚠️ Label is removed with every code change.

sonarqubecloud · 2024-01-11T13:39:40Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.6% Duplication on New Code

See analysis details on SonarCloud

Danielius1922 · 2024-01-11T17:49:46Z

@joochlee please check https://github.com/iotivity/iotivity-lite/actions/runs/7492383923/job/20395748989 , https://github.com/iotivity/iotivity-lite/actions/runs/7492383941/job/20395750617 and https://github.com/iotivity/iotivity-lite/actions/runs/7492383941/job/20395751298 there some documentation and printf formats compile errors

…_bridging

joochlee · 2024-01-12T07:39:04Z

PR #592 replaces this one

merge bridging

joochlee · 2024-01-15T12:47:01Z

I don't understand why code coverage of sonarcloud is so low..
How to pass this sonarCloud test?
Should I clear all code smells and security hot spots?
=> but some error is unfixable: e.g. "Make the type of this parameter a pointer-to-const. The current type of "user_data" is "void *" ".
=> If I fix this issue, build fails.. because of type mismatch. function pointer type should be changed to fix this issues..

Danielius1922 · 2024-01-15T13:38:46Z

I don't understand why code coverage of sonarcloud is so low.. How to pass this sonarCloud test? Should I clear all code smells and security hot spots? => but some error is unfixable: e.g. "Make the type of this parameter a pointer-to-const. The current type of "user_data" is "void *" ". => If I fix this issue, build fails.. because of type mismatch. function pointer type should be changed to fix this issues..

Yes, please try to fix the issues, we need SC to pass before we can merge. These are core files, so these need to be tested.

Ignore those type mismatch warnings, those are won't fix, I'll click those in the SC GUI (there is also usually other type of warning about that the programmer should use a concrete type instead of void*, those should be ignored as well).

For coverage, well new code needs new unit tests, no? Usually a .c file has a .cpp unittest file that tests it. So, for example you've added oc_uuid_is_nil to oc_uuid.c so you need to add unit test to api/unittest/uuidtest.cpp that tests that function. But you should probably wait until I review, because I might require a code rewrite and the tests would need to be rewritten as well. I'll try to check some files today, so you can continue your work. Right now oc_vod_map.c and oc_uiid.c look ok to me, so you can start by adding tests for new code there.

joochlee · 2024-01-15T14:18:36Z

do we need unittest for functions defined in "oc_*_internal.h" ?
It seems that it may not...

Danielius1922 · 2024-01-15T15:16:44Z

do we need unittest for functions defined in "oc_*_internal.h" ? It seems that it may not...

Well, it depends. Priority is to have public functions tested, so if the tests of a public function also test some internal function then it's not necessary to add separate tests for the internal function. However, sometimes it is easier to add unit tests for an internal function than setting up the tests of a parent function to hit the internal function.

joochlee · 2024-01-16T11:39:43Z

Hi, Is there any way to run specific test case ?
(for example, "ctest coreresourcetest -V" )

Danielius1922 · 2024-01-16T12:03:32Z

Hi, Is there any way to run specific test case ? (for example, "ctest coreresourcetest -V" )

Not sure if it is possible using ctest, but I do it by running the test binary with the --gtest_filter parameter.

For example, if you want to run CoreDevice_P from TestCoreResource then:

go to your build folder and you should have apitest binary (if not, you need to rebuild your project with the CMake switch -DBUILD_TESTING=ON)

run:

./apitest --gtest_filter="TestCoreResource.CoreDevice_P"

joochlee · 2024-01-16T12:12:58Z

Hi, Is there any way to run specific test case ? (for example, "ctest coreresourcetest -V" )

Not sure if it is possible using ctest, but I do it by running the test binary with the --gtest_filter parameter.

For example, if you want to run CoreDevice_P from TestCoreResource then:
go to your build folder and you should have apitest binary (if not, you need to rebuild your project with the CMake switch -DBUILD_TESTING=ON)
run:
./apitest --gtest_filter="TestCoreResource.CoreDevice_P"

Thank you !

joochlee · 2024-01-16T12:26:32Z

Another question.
what does "_P" and "_F" mean at the end of each test case name ??

Danielius1922 · 2024-01-16T12:35:02Z

Another question. what does "_P" and "_F" mean at the end of each test case name ??

The use is inconsistent in the repo, but I use it like this: "_P" (positive/passing path, meaning that the tested function succeeds) and "_F" (failure path, the tested function fails)

joochlee · 2024-01-16T12:36:00Z

Another question. what does "_P" and "_F" mean at the end of each test case name ??

The use is inconsistent in the repo, but I use it like this: "_P" (positive/passing path, meaning that the tested function succeeds) and "_F" (failure path, the tested function fails)

got it, Thanks !

.gitignore

CMakeLists.txt

api/oc_collection.c

include/oc_uuid.h

port/linux/ipadapter.c

…s, fix sonarCloud codesmells/security hot spots

Danielius1922 · 2024-01-26T09:26:38Z

api/oc_collection.c

@@ -609,6 +630,12 @@ oc_get_next_collection_with_link(const oc_resource_t *resource,
    collection = (oc_collection_t *)collection->res.next;
  }

+  /*
+   * FIXED <2024/01/23> oc_get_next_collection_with_link() : 


Yeah, seems like a bug. The original version seems to pressupose that the list of collections is ordered by device index, but this is not ensured by oc_collection_add when a new collection is added to the list.

Remove the old version and keep the fixed code.

Danielius1922 · 2024-01-26T09:41:47Z

api/oc_core_res.c

+#include "security/oc_ael_internal.h"
+#include "oc_acl.h"
+#include "oc_cred.h"
+#endif


#ifdef OC_HAS_FEATURE_BRIDGE #include "oc_acl.h" #include "oc_cred.h" #ifdef OC_SECURITY #include "security/oc_ael_internal.h" #include "security/oc_svr_internal.h" #endif /* OC_SECURITY*/ #endif /* OC_HAS_FEATURE_BRIDGE */

Thanks, it's much better!

Danielius1922 · 2024-01-26T09:43:40Z

api/oc_push.c

@@ -2300,7 +2301,9 @@ oc_push_free(void)
    _purge_recv_obj_list(recvs_instance);
    OC_PUSH_DBG("free push receiver Resource (device: %zu)... ",
                recvs_instance->resource->device);
+#if 0


Isn't the recvs_instance->resource leaking now?

Probably not.
recvs_instance->resource points Push Receiver Resource (oic.r.pushreceiver) which is one of App Resources.
All App Resources are removed by oc_ri_shutdown() as you know.
so I think that letting oc_ri_shutdown() do its own job is better idea.

what do you think ?

Danielius1922 · 2024-01-26T10:01:14Z

api/oc_core_res.c

@@ -105,6 +112,7 @@ oc_core_init(void)
 #endif /* OC_DYNAMIC_ALLOCATION */
 }

+#ifdef OC_DYNAMIC_ALLOCATION


This is a leak, oc_free_string has to be called even when dynamic allocation is disabled, because oc_strings use a shared byte pool buffer and without this deallocation the buffers aren't returned to the shared pool.

oc_core_free_device_info_properties() are called by 2 functions : oc_core_shutdown() and oc_core_remove_device_at_index().

When OC_DYNAMIC_ALLOCATION = OFF, oc_core_remove_device_at_index() and code block calling
oc_core_remove_device_at_index() of oc_core_shutdown() are disabled.

Therefore, If I remove #ifdef OC_DYNAMIC_ALLOCATION directive, oc_core_free_device_info_properties() is used by none. so gcc complains that this function is never used...

Danielius1922 · 2024-01-26T10:08:06Z

api/oc_core_res.c

+bool
+oc_core_remove_device_at_index(size_t index)
+{
+  if (index >= g_device_count) {


replace with oc_core_device_is_valid

Danielius1922 · 2024-01-26T10:39:04Z

api/oc_core_res.c

+  }
+
+#ifdef OC_SECURITY
+  oc_reset_device(index);


In some cases schedules delayed events - I see cloud deregistration and closing of TLS sessions for the device. We might be in for a nasty surprise when those events execute with a deleted device.

Danielius1922 · 2024-01-26T10:49:11Z

api/oc_ri.c

+    res = res->next;
+  }
+
+  oc_collections_free_per_device(index);


#ifdef OC_COLLECTIONS
oc_collections_free_per_device(index);
#endif /* OC_COLLECTIONS */

Danielius1922 · 2024-01-26T10:50:08Z

include/oc_ri.h

+ * @return non-NULL found resource
+ * @return NULL no more resource or end of list
+ */
+oc_resource_t *oc_ri_get_app_resource_by_device(size_t device, bool reset);


Do oc_ri_get_app_resource_by_device and oc_ri_delete_app_resources_per_device need to be part of the public API? I don't see them used in any apps.

oc_ri_get_app_resource_by_device() is used by bridge_manager module (not uploaded yet)
oc_ri_delete_app_resources_per_device() is used by oc_core_remove_device_at_index() in oc_core_res.c .

Danielius1922 · 2024-01-26T12:03:41Z

api/oc_ri.c

+oc_resource_t *
+oc_ri_get_app_resource_by_device(size_t device, bool reset)
+{
+  static oc_resource_t *rsc;


I'd copy the semantics of g_query_iterator, so that we are consistent.

typedef struct { oc_resource_t *rsc; #ifdef OC_COLLECTIONS oc_resource_t *coll_rsc; #endif } oc_ri_app_resource_iterator_t; static oc_ri_app_resource_iterator_t g_app_resource_iterator; void oc_ri_app_resource_iterator_init() { g_app_resource_iterator.rsc = oc_ri_get_app_resources(); #ifdef OC_COLLECTIONS g_app_resource_iterator.coll_rsc = oc_collection_get_collections(); #endif } oc_resource_t* oc_ri_app_resource_iterator_next_for_device(size_t device) { ... }

We also need to mention that deleting / adding a resource (collection or non-collection) invalidates the iterator.

Danielius1922 · 2024-01-26T12:05:43Z

include/oc_core_res.h

+
+#ifdef OC_HAS_FEATURE_BRIDGE
+  oc_string_t
+    ecoversion; ///< Version of ecosystem that a Bridged Device belongs to.


Remove. I don't see this used anywhere in this PR, add it when its functionality is added.

Danielius1922 · 2024-01-26T14:14:55Z

Renamed the feature branch to feature/ocfbridging , lets continue there and hopefully will bring the feature to master once all issues are solved.

joochlee and others added 20 commits July 9, 2023 16:05

- begin to merge ocfbridging to current master...

6d37797

modifyitn svr resource update codes

8e6dd3f

ongoing...

5b1e3d3

ongoing...

e9e158d

VOD를 추가 및 삭제할때 SVR을 처리해주는 부분 수정

cb3cac5

minor update

04d03e5

- new function: oc_bridge_print_device_list()

7d9d675

- bugfix: oc_ri_delete_app_resources_per_device() => fix infinite loop - remove function: oc_resource_set_data_remover() => not necessary any more

doxm_owned_changed(): make all VOD's oc_virtual_device_t.is_vod_onlin…

29cdb4c

…e = false oc_virtual_device_t : remove ecosystem_device field

oc_ri_get_app_resource_by_device() : make it search collection Resour…

bf4c1f8

…ce too

debugging for CTT

75dbb89

Merge branch 'bridging' into merge_bridging

1b53a9f

add .project file

86b4127

api/unittest/vodmaptest.cpp : GTest is completed

2883b04

merge bridging feature

bridging : clean up codes

edd7610

bridging : add .md file, update Makefile to enable bridging feature, …

17908bb

…clean up codes

merge_bridging : restore original contents of "sonar-project.properties"

520944c

merge_bridging : remove debug codes

768d827

merge_bridging : typo fix

ee26341

Merge branch 'joochlee/feature/merge_bridging' into merge_bridging

aab6050

Merge pull request #584 from joochlee/merge_bridging

bcb3671

Merge bridging

Enable briding in GitHub workflows

ff06343

joochlee added 2 commits January 12, 2024 15:59

merge_bridging : fix size_t formatter, doxygen keyword issues

4377ec7

Merge remote-tracking branch 'gh_ssh_jclee/merge_bridging' into merge…

6813779

…_bridging

joochlee closed this Jan 12, 2024

github-actions bot locked and limited conversation to collaborators Jan 12, 2024

merge_bridging : fix code analyzation results

cde04f1

joochlee and others added 4 commits January 13, 2024 23:39

merge_bridging : clean up codes

c8edd3d

merge_bridging : clean up codes

7249cfa

merge_bridging : fixed sonarcloud issue

0adf0d3

Merge pull request #594 from joochlee/joochlee/feature/merge_bridging

0e61bdd

merge bridging

Danielius1922 commented Jan 16, 2024

View reviewed changes

Merge branch 'master' into joochlee/feature/merge_bridging

4d54fc0

Danielius1922 force-pushed the joochlee/feature/merge_bridging branch from 630b2de to 4d54fc0 Compare January 26, 2024 08:56

joochlee added 10 commits January 26, 2024 10:10

merge_bridging: revert some changes, add unittest for most of new API…

1aac6bf

…s, fix sonarCloud codesmells/security hot spots

merge_briding : fix clang-tidy issue

ba78f47

merge_bridging : fix conditional build issue

7d6b47b

merge_bridging : fix unittest issues

0ed25ce

merge_bridging: fix unittest issues

6c157f6

merge_bridging : fix unittest issues

822328e

merge_bridging : fix unittest issues

a309cc9

merge_bridging : fix unittest issues

371ab70

merge_bridging : fix unittest issues

e8c446b

merge_bridging : fix unittest issues

316f1c5

Danielius1922 commented Jan 26, 2024

View reviewed changes

Danielius1922 closed this Jan 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCF Bridging #588

OCF Bridging #588

Danielius1922 commented Jan 11, 2024

ocf-conformance-test-tool bot commented Jan 11, 2024

sonarqubecloud bot commented Jan 11, 2024

Danielius1922 commented Jan 11, 2024

joochlee commented Jan 12, 2024

joochlee commented Jan 15, 2024 •

edited

Loading

Danielius1922 commented Jan 15, 2024 •

edited

Loading

joochlee commented Jan 15, 2024

Danielius1922 commented Jan 15, 2024

joochlee commented Jan 16, 2024 •

edited

Loading

Danielius1922 commented Jan 16, 2024

joochlee commented Jan 16, 2024

joochlee commented Jan 16, 2024

Danielius1922 commented Jan 16, 2024 •

edited

Loading

joochlee commented Jan 16, 2024

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024 •

edited

Loading

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024 •

edited

Loading

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024

Danielius1922 Jan 26, 2024

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024

Danielius1922 Jan 26, 2024

Danielius1922 Jan 26, 2024

joochlee Jan 26, 2024

Danielius1922 commented Jan 26, 2024

OCF Bridging #588

OCF Bridging #588

Conversation

Danielius1922 commented Jan 11, 2024

ocf-conformance-test-tool bot commented Jan 11, 2024

sonarqubecloud bot commented Jan 11, 2024

Quality Gate passed

Danielius1922 commented Jan 11, 2024

joochlee commented Jan 12, 2024

joochlee commented Jan 15, 2024 • edited Loading

Danielius1922 commented Jan 15, 2024 • edited Loading

joochlee commented Jan 15, 2024

Danielius1922 commented Jan 15, 2024

joochlee commented Jan 16, 2024 • edited Loading

Danielius1922 commented Jan 16, 2024

joochlee commented Jan 16, 2024

joochlee commented Jan 16, 2024

Danielius1922 commented Jan 16, 2024 • edited Loading

joochlee commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joochlee Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joochlee Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Danielius1922 commented Jan 26, 2024

joochlee commented Jan 15, 2024 •

edited

Loading

Danielius1922 commented Jan 15, 2024 •

edited

Loading

joochlee commented Jan 16, 2024 •

edited

Loading

Danielius1922 commented Jan 16, 2024 •

edited

Loading

joochlee Jan 26, 2024 •

edited

Loading

joochlee Jan 26, 2024 •

edited

Loading