[devel] [PATCH 1/1] mds: fix memleak in code and test [#1860]
--- src/mds/apitest/mdstipc.h | 2 +- src/mds/apitest/mdstipc_api.c | 134 +++-- src/mds/apitest/mdstipc_conf.c | 9 ++- src/mds/mds_c_sndrcv.c | 1 + src/mds/mds_tipc_fctrl_intf.cc | 4 +- 5 files changed, 88 insertions(+), 62 deletions(-) diff --git a/src/mds/apitest/mdstipc.h b/src/mds/apitest/mdstipc.h index 5fd7b9c6e..b56940ea6 100644 --- a/src/mds/apitest/mdstipc.h +++ b/src/mds/apitest/mdstipc.h @@ -203,7 +203,7 @@ uint32_t destroy_pwe_on_vdest(MDS_HDL); /** USER DEFINED WRAPPERS FOR MDS SERVICE APIs **/ -uint32_t tet_create_task(NCS_OS_CB, NCSCONTEXT); +uint32_t tet_create_task(NCS_OS_CB, NCSCONTEXT*); uint32_t tet_release_task(void *task_handle); int is_adest_sel_obj_found(int); int is_sel_obj_found(int); diff --git a/src/mds/apitest/mdstipc_api.c b/src/mds/apitest/mdstipc_api.c index 651365e95..847f9a7f1 100644 --- a/src/mds/apitest/mdstipc_api.c +++ b/src/mds/apitest/mdstipc_api.c @@ -398,7 +398,7 @@ void tet_svc_install_tp_10() printf( "\nTest case 10:Installing the External MIN service EXTMIN in a seperate thread and Uninstalling it here\n"); // Install thread - rc = tet_create_task((NCS_OS_CB)tet_vdest_install_thread, t_handle); + rc = tet_create_task((NCS_OS_CB)tet_vdest_install_thread, &t_handle); if (rc != NCSCC_RC_SUCCESS) { printf("\nFail to Install thread\n"); FAIL = 1; @@ -999,7 +999,7 @@ void tet_svc_unstall_tp_5() // Uninstalling the above service in a seperate thread // Uninstall thread rc = tet_create_task((NCS_OS_CB)tet_vdest_uninstall_thread, -gl_tet_vdest[0].svc[0].task.t_handle); +&gl_tet_vdest[0].svc[0].task.t_handle); if (rc != NCSCC_RC_SUCCESS) { printf("\nFail to create the uninstall thread\n"); FAIL = 1; @@ -2141,12 +2141,18 @@ void cleanup_ADEST_srv() { int id; printf("\nUninstalling all the services on this ADESt\n"); - for (id = gl_tet_adest.svc_count - 1; id >= 0; id--) + for (id = gl_tet_adest.svc_count - 1; id >= 0; id--) { + if (mds_service_retrieve(gl_tet_adest.mds_pwe1_hdl, +gl_tet_adest.svc[id].svc_id, +SA_DISPATCH_ALL) != NCSCC_RC_SUCCESS) { + printf("Adest Svc Retrieve Fail\n"); + } if (mds_service_uninstall(gl_tet_adest.mds_pwe1_hdl, gl_tet_adest.svc[id].svc_id) != NCSCC_RC_SUCCESS) { printf("\nFail mds_service_uninstall\n"); } + } } void tet_svc_subscr_ADEST_1() @@ -2441,7 +2447,7 @@ void tet_svc_subscr_ADEST_8() } printf("\nAction: Cancel in a seperate thread\n"); if (tet_create_task((NCS_OS_CB)tet_adest_cancel_thread, - gl_tet_adest.svc[0].task.t_handle) == + &gl_tet_adest.svc[0].task.t_handle) == NCSCC_RC_SUCCESS) { printf("\nTask has been Created\n"); fflush(stdout); @@ -2547,7 +2553,7 @@ void tet_svc_subscr_ADEST_10() printf("\nAction: Retrieve in a seperate thread\n"); /*Retrieve thread*/ if (tet_create_task((NCS_OS_CB)tet_adest_retrieve_thread, - gl_tet_adest.svc[0].task.t_handle) == + &gl_tet_adest.svc[0].task.t_handle) == NCSCC_RC_SUCCESS) { printf("\nTask has been Created\n"); fflush(stdout); @@ -2751,7 +2757,10 @@ uint32_t tet_cleanup_setup() printf("Fail mds_service_retrieve\n"); FAIL = 1; } - + if (gl_rcvdmsginfo.msg) { + free(gl_rcvdmsginfo.msg); + gl_rcvdmsginfo.msg = NULL; + } if (mds_service_uninstall( gl_tet_vdest[i].mds_pwe1_hdl, gl_tet_vdest[i].svc[id].svc_id) != @@ -2785,6 +2794,10 @@ uint32_t tet_cleanup_setup() printf("Adest Svc Retrieve Fail\n"); FAIL = 1; } + if (gl_rcvdmsginfo.msg) { + free(gl_rcvdmsginfo.msg); + gl_rcvdmsginfo.msg = NULL; + } if (mds_service_uninstall(gl_tet_adest.mds_pwe1_hdl, i) != NCSCC_RC_SUCCESS) { printf("Adest Svc Uninstall Fail\n"); @@ -2800,6 +2813,10 @@ uint32_t tet_cleanup_setup() print
[devel] [PATCH 0/1] Review Request for mds: fix memleak in code and test [#1860]
Summary: mds: fix memleak in code and test [#1860] Review request for Ticket(s): 1860 Peer Reviewer(s): Minh, Vu, Gary Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-1860 Base revision: 1aaa913e028197cc4fd6cd77023b3830388cd9c9 Personal repository: git://git.code.sf.net/u/thuantr/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries y Samples n Tests y Other n NOTE: Patch(es) contain lines longer than 80 characers Comments (indicate scope for each "y" above): - N/A revision 0b2d42d0a5bc8a7f69489aa6f8b4f3948b685f1d Author: thuan.tran Date: Tue, 19 Nov 2019 13:41:53 +0700 mds: fix memleak in code and test [#1860] Complete diffstat: -- src/mds/apitest/mdstipc.h | 2 +- src/mds/apitest/mdstipc_api.c | 134 +++-- src/mds/apitest/mdstipc_conf.c | 9 ++- src/mds/mds_c_sndrcv.c | 1 + src/mds/mds_tipc_fctrl_intf.cc | 4 +- 5 files changed, 88 insertions(+), 62 deletions(-) Testing Commands: - N/A Testing, Expected Results: -- N/A Conditions of Submission: - ACK by reviewers Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] imm: Fix coding issues identified by codechecker [#3115]
Hi Vu, I used osaf_clock_gettime() since it's used some places already in current code file. E.g: Line 626, Line 638. I think we keep use it to easy for code reading. Or we have to change all osaf_clock_gettime() calls in current code file. Best Regards, ThuanTr -Original Message- From: Nguyen Minh Vu Sent: Tuesday, November 19, 2019 10:01 AM To: thuan.tran ; 'Minh Hon Chau' ; gary@dektech.com.au Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1/1] imm: Fix coding issues identified by codechecker [#3115] Hi Thuan, Ack with a minor comment. Regards, On 11/4/19 2:57 PM, thuan.tran wrote: > --- > src/imm/agent/imma_db.cc | 2 +- > src/imm/immnd/immnd_main.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/imm/agent/imma_db.cc b/src/imm/agent/imma_db.cc > index 071edbe74..80637e55f 100644 > --- a/src/imm/agent/imma_db.cc > +++ b/src/imm/agent/imma_db.cc > @@ -621,7 +621,7 @@ int imma_oi_ccb_record_note_callback(IMMA_CLIENT_NODE > *cl_node, > rs = 1; > } > } > - if (callback) { > + if (tmp && callback) { > if (callback->type == IMMA_CALLBACK_OI_CCB_CREATE && > !(tmp->adminOwner)) { > SaImmAttrValuesT_2 **attributes = > (SaImmAttrValuesT_2 **)callback->attrValsForCreateUc; > diff --git a/src/imm/immnd/immnd_main.c b/src/imm/immnd/immnd_main.c > index 5280f0599..62c7b2478 100644 > --- a/src/imm/immnd/immnd_main.c > +++ b/src/imm/immnd/immnd_main.c > @@ -489,6 +489,7 @@ int main(int argc, char *argv[]) > fds[FD_CLM_INIT].fd = immnd_cb->clm_init_sel_obj.rmv_obj; > fds[FD_CLM_INIT].events = POLLIN; > > + osaf_clock_gettime(CLOCK_MONOTONIC, &start_time); [Vu] Is it better to use the one provided by base as below? struct timespec start_time = base::ReadMonotonicClock() > while (1) { > /* Watch out for performance bug. Possibly change from > event-count to recalculated timer. */ ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] imm: Fix coding issues identified by codechecker [#3115]
Hi Thuan, Ack with a minor comment. Regards, On 11/4/19 2:57 PM, thuan.tran wrote: --- src/imm/agent/imma_db.cc | 2 +- src/imm/immnd/immnd_main.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/imm/agent/imma_db.cc b/src/imm/agent/imma_db.cc index 071edbe74..80637e55f 100644 --- a/src/imm/agent/imma_db.cc +++ b/src/imm/agent/imma_db.cc @@ -621,7 +621,7 @@ int imma_oi_ccb_record_note_callback(IMMA_CLIENT_NODE *cl_node, rs = 1; } } - if (callback) { + if (tmp && callback) { if (callback->type == IMMA_CALLBACK_OI_CCB_CREATE && !(tmp->adminOwner)) { SaImmAttrValuesT_2 **attributes = (SaImmAttrValuesT_2 **)callback->attrValsForCreateUc; diff --git a/src/imm/immnd/immnd_main.c b/src/imm/immnd/immnd_main.c index 5280f0599..62c7b2478 100644 --- a/src/imm/immnd/immnd_main.c +++ b/src/imm/immnd/immnd_main.c @@ -489,6 +489,7 @@ int main(int argc, char *argv[]) fds[FD_CLM_INIT].fd = immnd_cb->clm_init_sel_obj.rmv_obj; fds[FD_CLM_INIT].events = POLLIN; + osaf_clock_gettime(CLOCK_MONOTONIC, &start_time); [Vu] Is it better to use the one provided by base as below? struct timespec start_time = base::ReadMonotonicClock() while (1) { /* Watch out for performance bug. Possibly change from event-count to recalculated timer. */ ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] log: Fix coding issues identified by codechecker [#3113]
Hi Vu, Agree, please help update NULL to nullptr before push. Thanks. Best Regards, ThuanTr -Original Message- From: Nguyen Minh Vu Sent: Tuesday, November 19, 2019 9:58 AM To: thuan.tran ; 'Minh Hon Chau' ; gary@dektech.com.au Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [PATCH 1/1] log: Fix coding issues identified by codechecker [#3113] Hi Thuan, Ack with a minor comment. Regards, Vu On 11/4/19 2:17 PM, thuan.tran wrote: > --- > src/log/logd/lgs_mbcsv.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/log/logd/lgs_mbcsv.cc b/src/log/logd/lgs_mbcsv.cc > index cd3d70009..ebc659ea1 100644 > --- a/src/log/logd/lgs_mbcsv.cc > +++ b/src/log/logd/lgs_mbcsv.cc > @@ -1931,7 +1931,7 @@ static uint32_t ckpt_proc_log_write(lgs_cb_t *cb, void > *data) { > /* If configured for split file system log records shall be written also > if > * we are standby. > */ > - if (lgs_is_split_file_system()) { > + if (lgs_is_split_file_system() && (logRecord != NULL)) { [Vu] Prefer using nullptr to NULL > size_t rec_len = strlen(logRecord); > stream->act_last_close_timestamp = c_file_close_time_stamp; > ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] log: Fix coding issues identified by codechecker [#3113]
Hi Thuan, Ack with a minor comment. Regards, Vu On 11/4/19 2:17 PM, thuan.tran wrote: --- src/log/logd/lgs_mbcsv.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/log/logd/lgs_mbcsv.cc b/src/log/logd/lgs_mbcsv.cc index cd3d70009..ebc659ea1 100644 --- a/src/log/logd/lgs_mbcsv.cc +++ b/src/log/logd/lgs_mbcsv.cc @@ -1931,7 +1931,7 @@ static uint32_t ckpt_proc_log_write(lgs_cb_t *cb, void *data) { /* If configured for split file system log records shall be written also if * we are standby. */ - if (lgs_is_split_file_system()) { + if (lgs_is_split_file_system() && (logRecord != NULL)) { [Vu] Prefer using nullptr to NULL size_t rec_len = strlen(logRecord); stream->act_last_close_timestamp = c_file_close_time_stamp; ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] ntf: Fix coding issues identified by codechecker [#3114]
Hi Thuan ack from me. Thanks Minh On 4/11/19 6:42 pm, thuan.tran wrote: --- src/ntf/agent/ntfa_api.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/ntf/agent/ntfa_api.c b/src/ntf/agent/ntfa_api.c index 417c9d688..e89479bf6 100644 --- a/src/ntf/agent/ntfa_api.c +++ b/src/ntf/agent/ntfa_api.c @@ -1379,30 +1379,31 @@ SaAisErrorT recoverClient(ntfa_client_hdl_rec_t *client_hdl) if ((rc = reinitializeClient(client_hdl)) == SA_AIS_OK) { /* Restore reader */ ntfa_reader_hdl_rec_t *reader_hdl = client_hdl->reader_list; - while (reader_hdl != NULL && rc == SA_AIS_OK) { + while (reader_hdl != NULL) { rc = recoverReader(client_hdl, reader_hdl); + if (rc != SA_AIS_OK) { + TRACE("Failed to restore reader (readerId:%d)", + reader_hdl->reader_id); + goto done; + } reader_hdl = reader_hdl->next; } - if (rc != SA_AIS_OK) { - TRACE("Failed to restore reader (readerId:%d)", - reader_hdl->reader_id); - goto done; - } /* Restore subscriber */ ntfa_subscriber_list_t *subscriber_hdl = subscriberNoList; - while (subscriber_hdl != NULL && rc == SA_AIS_OK) { + while (subscriber_hdl != NULL) { if (client_hdl->local_hdl == - subscriber_hdl->subscriberListNtfHandle) + subscriber_hdl->subscriberListNtfHandle) { rc = recoverSubscriber(client_hdl, subscriber_hdl); + if (rc != SA_AIS_OK) { + TRACE( + "Failed to restore subscriber (subscriptionId:%d)", + subscriber_hdl->subscriberListSubscriptionId); + goto done; + } + } subscriber_hdl = subscriber_hdl->next; } - if (rc != SA_AIS_OK) { - TRACE( - "Failed to restore subscriber (subscriptionId:%d)", - subscriber_hdl->subscriberListSubscriptionId); - goto done; - } client_hdl->valid = true; } else { TRACE("Failed to restore client (id:%d)", ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] mds: Fix coding issues identified by codechecker [#3112]
Hi Thuan ack from me. thanks Minh On 4/11/19 5:56 pm, thuan.tran wrote: --- src/mds/mds_c_db.c | 1 + src/mds/mds_c_sndrcv.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mds/mds_c_db.c b/src/mds/mds_c_db.c index 58f0e3aee..e1991517e 100644 --- a/src/mds/mds_c_db.c +++ b/src/mds/mds_c_db.c @@ -433,6 +433,7 @@ uint32_t mds_vdest_tbl_get_role(MDS_VDEST_ID vdest_id, V_DEST_RL *role) vdest_info = (MDS_VDEST_INFO *)ncs_patricia_tree_get( &gl_mds_mcm_cb->vdest_list, (uint8_t *)&vdest_id); if (vdest_info == NULL) { + *role = V_DEST_RL_INVALID; m_MDS_LOG_DBG("MDS:DB: VDEST not present"); m_MDS_LEAVE(); return NCSCC_RC_FAILURE; diff --git a/src/mds/mds_c_sndrcv.c b/src/mds/mds_c_sndrcv.c index 7850ac714..0dc76eef4 100644 --- a/src/mds/mds_c_sndrcv.c +++ b/src/mds/mds_c_sndrcv.c @@ -2319,7 +2319,7 @@ static uint32_t mcm_query_for_node_dest(MDS_DEST adest, uint8_t *to) *to = DESTINATION_SAME_PROCESS; else *to = DESTINATION_ON_NODE; - } else if (dest_node_id != src_node_id) { + } else { *to = DESTINATION_OFF_NODE; } return NCSCC_RC_SUCCESS; ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel