[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, _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);
+_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) ==
+ _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) ==
+ _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()
printf("Adest
[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, _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, _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(
_mds_mcm_cb->vdest_list, (uint8_t *)_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
