Re: [devel] [PATCH 1/1] rde: correct to promote node to active [#3108]
Hi
Ack (tested)
-Original Message-
From: thang.d.nguyen [mailto:thang.d.ngu...@dektech.com.au]
Sent: Tuesday, 4 February 2020 1:37 PM
To: Gary Lee
Cc: opensaf-devel@lists.sourceforge.net; Thang Duc Nguyen
Subject: [PATCH 1/1] rde: correct to promote node to active [#3108]
If relaxed node promotion is enabled, allow this node to be promoted active
if it can see a peer SC and this node has the lowest node ID.
---
src/rde/rded/role.cc | 14 +- src/rde/rded/role.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/rde/rded/role.cc b/src/rde/rded/role.cc index
593ccf0eb..7ca020d5d 100644
--- a/src/rde/rded/role.cc
+++ b/src/rde/rded/role.cc
@@ -260,7 +260,8 @@ bool Role::IsCandidate() {
// if relaxed node promotion is enabled, allow this node to be promoted
// active if it can see a peer SC and this node has the lowest node ID
if (consensus_service.IsRelaxedNodePromotionEnabled() == true &&
- cb->state == State::kNotActiveSeenPeer) {
+ cb->state == State::kNotActiveSeenPeer &&
+ IsLowestNodeid() == true) {
LOG_NO("Relaxed node promotion enabled. This node is a candidate.");
result = true;
}
@@ -279,6 +280,17 @@ bool Role::IsPeerPresent() {
return result;
}
+bool Role::IsLowestNodeid() {
+ bool result = true;
+ RDE_CONTROL_BLOCK* cb = rde_get_control_block();
+
+ for (auto peer_id : cb->peer_controllers) {
+if (peer_id < own_node_id_)
+ return false;
+ }
+ return result;
+}
+
uint32_t Role::SetRole(PCS_RDA_ROLE new_role) {
TRACE_ENTER();
PCS_RDA_ROLE old_role = role_;
diff --git a/src/rde/rded/role.h b/src/rde/rded/role.h index
9c63cbe7b..9bf1b10bd 100644
--- a/src/rde/rded/role.h
+++ b/src/rde/rded/role.h
@@ -38,6 +38,7 @@ class Role {
void AddPeer(NODE_ID node_id);
bool IsCandidate();
bool IsPeerPresent();
+ bool IsLowestNodeid();
void SetPeerState(PCS_RDA_ROLE node_role, NODE_ID node_id);
timespec* Poll(timespec* ts);
uint32_t SetRole(PCS_RDA_ROLE new_role);
--
2.17.1
smime.p7s
Description: S/MIME cryptographic signature
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]
Hi Alex,
OK, you can keep strncpy with len + 1.
No more comment from me.
From: Alex Jones
Sent: Tuesday, February 4, 2020 9:40 PM
To: Thuan Tran ; Vu Minh Nguyen
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]
Hi ThuanTr,
I will add fclose(). Good catch.
We can't leave the original code in SmfUtils.cc because it fails to compile
in gcc 9.x. The compiler complains that you are only copying the length of the
string, so the output is not null terminated (even though the next line null
terminates it). We could change the code to use memcpy instead. That would make
it clearer that we are not intending to null terminate with the function call,
and are doing it ourselves in the next line.
Alex
On 2/3/20 9:28 PM, Tran Thuan wrote:
NOTICE: This email was received from an EXTERNAL sender
Hi Alex,
About test_ntf_imcn.cc, please update following too
Since you add “return” then static code check report leak “ f ”.
@@ -6202,6 +6202,7 @@ __attribute__((constructor)) static void
ntf_imcn_constructor(void) {
snprintf(cp_cmd, sizeof(cp_cmd), "cp ");
if ((strlen(line) - 1) > (sizeof(cp_cmd) - sizeof("cp "))) {
printf("line: %s too long", line);
+ fclose(f);
return;
}
About SmfUtils.cc:
- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';
=> strncpy with “len + 1” then later overwrite with ‘\0’.
I suggest strncpy with “len” as original code to avoid redundant changes.
Best Regards,
ThuanTr
From: Alex Jones
Sent: Monday, February 3, 2020 10:39 PM
To: thuan.t...@dektech.com.au;
vu.m.ngu...@dektech.com.au
Cc:
opensaf-devel@lists.sourceforge.net;
Alex Jones
Subject: [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]
Rework fixes in NTF and SMF.
---
src/ntf/apitest/test_ntf_imcn.cc | 2 +-
src/smf/smfd/SmfUtils.cc | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/ntf/apitest/test_ntf_imcn.cc b/src/ntf/apitest/test_ntf_imcn.cc
index 51b9076c6..04f155074 100644
--- a/src/ntf/apitest/test_ntf_imcn.cc
+++ b/src/ntf/apitest/test_ntf_imcn.cc
@@ -1140,7 +1140,7 @@ static SaAisErrorT set_add_info(
>additionalInfo[idx].infoValue);
if (error == SA_AIS_OK) {
strcpy(reinterpret_cast(temp), infoValue);
- temp[strlen(infoValue) - 1] = '\0';
+ //temp[strlen(infoValue)] = '\0';
nHeader->additionalInfo[idx].infoId = infoId;
nHeader->additionalInfo[idx].infoType = SA_NTF_VALUE_STRING;
}
diff --git a/src/smf/smfd/SmfUtils.cc b/src/smf/smfd/SmfUtils.cc
index 2d539e7c2..f1593b4cf 100644
--- a/src/smf/smfd/SmfUtils.cc
+++ b/src/smf/smfd/SmfUtils.cc
@@ -993,7 +993,7 @@ bool smf_stringToValue(SaImmValueTypeT i_type,
SaImmAttrValueT *i_value,
len = strlen(i_str);
*i_value = malloc(sizeof(SaStringT));
*((SaStringT *)*i_value) = (SaStringT)malloc(len + 1);
- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';
break;
case SA_IMM_ATTR_SAANYT:
--
2.21.1
Notice: This e-mail together with any attachments may contain information of
Ribbon Communications Inc. that is confidential and/or proprietary for the sole
use of the intended recipient. Any review, disclosure, reliance or distribution
by others or forwarding without express permission is strictly prohibited. If
you are not the intended recipient, please notify the sender immediately and
then delete all copies, including any attachments.
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]
Hi ThuanTr,
I will add fclose(). Good catch.
We can't leave the original code in SmfUtils.cc because it fails to
compile in gcc 9.x. The compiler complains that you are only copying
the length of the string, so the output is not null terminated (even
though the next line null terminates it). We could change the code to
use memcpy instead. That would make it clearer that we are not
intending to null terminate with the function call, and are doing it
ourselves in the next line.
Alex
On 2/3/20 9:28 PM, Tran Thuan wrote:
__
NOTICE: This email was received from an EXTERNAL sender
__
Hi Alex,
About test_ntf_imcn.cc, please update following too
Since you add "return" then static code check report leak " f ".
@@ -6202,6 +6202,7 @@ __attribute__((constructor)) static void
ntf_imcn_constructor(void) {
snprintf(cp_cmd, sizeof(cp_cmd), "cp ");
if ((strlen(line) - 1) > (sizeof(cp_cmd) - sizeof("cp ")))
{
printf("line: %s too long", line);
+ fclose(f);
return;
}
About SmfUtils.cc:
- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';
=> strncpy with "len + 1" then later overwrite with `\0'.
I suggest strncpy with "len" as original code to avoid redundant
changes.
Best Regards,
ThuanTr
From: Alex Jones [1]
Sent: Monday, February 3, 2020 10:39 PM
To: [2]thuan.t...@dektech.com.au; [3]vu.m.ngu...@dektech.com.au
Cc: [4]opensaf-devel@lists.sourceforge.net; Alex Jones
[5]
Subject: [PATCH 5/5] build: fix compile errors with gcc 9.x [#3134]
Rework fixes in NTF and SMF.
---
src/ntf/apitest/test_ntf_imcn.cc | 2 +-
src/smf/smfd/SmfUtils.cc | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/ntf/apitest/test_ntf_imcn.cc
b/src/ntf/apitest/test_ntf_imcn.cc
index 51b9076c6..04f155074 100644
--- a/src/ntf/apitest/test_ntf_imcn.cc
+++ b/src/ntf/apitest/test_ntf_imcn.cc
@@ -1140,7 +1140,7 @@ static SaAisErrorT set_add_info(
>additionalInfo[idx].infoValue);
if (error == SA_AIS_OK) {
strcpy(reinterpret_cast(temp), infoValue);
- temp[strlen(infoValue) - 1] = '\0';
+ //temp[strlen(infoValue)] = '\0';
nHeader->additionalInfo[idx].infoId = infoId;
nHeader->additionalInfo[idx].infoType = SA_NTF_VALUE_STRING;
}
diff --git a/src/smf/smfd/SmfUtils.cc b/src/smf/smfd/SmfUtils.cc
index 2d539e7c2..f1593b4cf 100644
--- a/src/smf/smfd/SmfUtils.cc
+++ b/src/smf/smfd/SmfUtils.cc
@@ -993,7 +993,7 @@ bool smf_stringToValue(SaImmValueTypeT i_type,
SaImmAttrValueT *i_value,
len = strlen(i_str);
*i_value = malloc(sizeof(SaStringT));
*((SaStringT *)*i_value) = (SaStringT)malloc(len + 1);
- strncpy(*((SaStringT *)*i_value), i_str, len - 1);
+ strncpy(*((SaStringT *)*i_value), i_str, len + 1);
(*((SaStringT *)*i_value))[len] = '\0';
break;
case SA_IMM_ATTR_SAANYT:
--
2.21.1
___
Notice: This e-mail together with any attachments may contain
information of Ribbon Communications Inc. that is confidential and/or
proprietary for the sole use of the intended recipient. Any review,
disclosure, reliance or distribution by others or forwarding without
express permission is strictly prohibited. If you are not the intended
recipient, please notify the sender immediately and then delete all
copies, including any attachments.
___
References
1. mailto:ajo...@rbbn.com
2. mailto:thuan.t...@dektech.com.au
3. mailto:vu.m.ngu...@dektech.com.au
4. mailto:opensaf-devel@lists.sourceforge.net
5. mailto:ajo...@rbbn.com
0x0023444D652FA1D5.asc
Description: application/pgp-keys
signature.asc
Description: OpenPGP digital signature
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] mds: ignore flow control message on disabled FCTRL agent [#3148]
- The confused error in mds log since disabled FCTRL agent
receive a flow control message type from enabled FCTRL agent.
MDS should ignore flow control message on disabled FCTRL agent.
- Refactor mds_tipc_fctrl_drop_data() for code consistency.
- Also print Adest info detail in logging for easy troubleshoot.
---
src/mds/mds_dt_common.c| 65 ++
src/mds/mds_tipc_fctrl_intf.cc | 63 +---
2 files changed, 69 insertions(+), 59 deletions(-)
diff --git a/src/mds/mds_dt_common.c b/src/mds/mds_dt_common.c
index ef8425cb9..6f1bd8a2e 100644
--- a/src/mds/mds_dt_common.c
+++ b/src/mds/mds_dt_common.c
@@ -314,9 +314,11 @@ uint32_t mdtm_process_recv_message_common(uint8_t flag,
uint8_t *buffer,
* header length */
if (len < len_mds_hdr) {
m_MDS_LOG_ERR(
- "MDTM: Message recd (Non Fragmented) len is less
than the MDS HDR len Adest = <%" PRId64
- "> len =%d len_mds_hdr=%d",
- transport_adest, len, len_mds_hdr);
+ "MDTM: Message recd (Non Fragmented) len is less
than the MDS HDR len "
+ "Adest <0x%08x, %u> len =%d len_mds_hdr=%d",
+ m_MDS_GET_NODE_ID_FROM_ADEST(transport_adest),
+ m_MDS_GET_PROCESS_ID_FROM_ADEST(transport_adest),
+ len, len_mds_hdr);
return NCSCC_RC_FAILURE;
}
@@ -350,10 +352,10 @@ uint32_t mdtm_process_recv_message_common(uint8_t flag,
uint8_t *buffer,
_svc_hdl)) {
*buff_dump = 0; /* For future hack */
m_MDS_LOG_ERR(
- "MDTM: svc_id = %s(%d) Doesnt exists for the
message recd, Adest = <%" PRId64
- ">\n",
+ "MDTM: svc_id = %s(%d) Doesnt exists for the
message recd, Adest <0x%08x, %u>\n",
get_svc_names(dest_svc_id), dest_svc_id,
- transport_adest);
+ m_MDS_GET_NODE_ID_FROM_ADEST(transport_adest),
+ m_MDS_GET_PROCESS_ID_FROM_ADEST(transport_adest));
return NCSCC_RC_FAILURE;
}
@@ -365,9 +367,9 @@ uint32_t mdtm_process_recv_message_common(uint8_t flag,
uint8_t *buffer,
if (enc_type > MDS_ENC_TYPE_DIRECT_BUFF) {
*buff_dump = 0; /* For future hack */
m_MDS_LOG_ERR(
- "MDTM: Encoding unsupported, Adest = <%" PRId64
- ">\n",
- transport_adest);
+ "MDTM: Encoding unsupported, Adest Adest <0x%08x,
%u>\n",
+ m_MDS_GET_NODE_ID_FROM_ADEST(transport_adest),
+ m_MDS_GET_PROCESS_ID_FROM_ADEST(transport_adest));
return NCSCC_RC_FAILURE;
}
@@ -506,9 +508,10 @@ uint32_t mdtm_process_recv_message_common(uint8_t flag,
uint8_t *buffer,
}
m_MDS_LOG_DBG(
- "MDTM: Recd Unfragmented message with SVC Seq num =%d, from
src Adest = <%" PRId64
- ">",
- svc_seq_num, transport_adest);
+ "MDTM: Recd Unfragmented message with SVC Seq num =%d, from
src Adest <0x%08x, %u>",
+ svc_seq_num,
+ m_MDS_GET_NODE_ID_FROM_ADEST(transport_adest),
+ m_MDS_GET_PROCESS_ID_FROM_ADEST(transport_adest));
if (msg_snd_type == MDS_SENDTYPE_ACK) {
/* NOTE: Version in ACK message is ignored */
@@ -603,9 +606,9 @@ uint32_t mdtm_process_recv_message_common(uint8_t flag,
uint8_t *buffer,
if (len <= (len_mds_hdr + MDTM_FRAG_HDR_LEN)) {
m_MDS_LOG_ERR(
"MDTM: Message recd (Fragmented First Pkt) len is
less than or equal to \
-the sum of (len_mds_hdr+MDTM_FRAG_HDR_LEN) len,
Adest = <%" PRId64
- ">",
- transport_adest);
+ the sum of (len_mds_hdr+MDTM_FRAG_HDR_LEN) len,
Adest <0x%08x, %u>",
+ m_MDS_GET_NODE_ID_FROM_ADEST(transport_adest),
+ m_MDS_GET_PROCESS_ID_FROM_ADEST(transport_adest));
return NCSCC_RC_FAILURE;
}
data = [MDS_HEADER_PWE_ID_POSITION + MDTM_FRAG_HDR_LEN];
@@ -796,9 +799,10 @@ uint32_t mdtm_process_recv_message_common(uint8_t flag,
uint8_t *buffer,
m_MDS_LOG_INFO("MDTM: Reassembly started\n");
m_MDS_LOG_DBG(
- "MDTM: Recd fragmented message(first frag) with Frag
[devel] [PATCH 0/1] Review Request for mds: ignore flow control message on disabled FCTRL agent [#3148] V2
Summary: mds: ignore flow control message on disabled FCTRL agent [#3148] Review request for Ticket(s): 3148 Peer Reviewer(s): Thang, Vu, Minh, Gary Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3148 Base revision: 876fbce762044d49da8edbd6bfcb059ee59e748e 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 n Other n NOTE: Patch(es) contain lines longer than 80 characers Comments (indicate scope for each "y" above): - N/A revision 57a71b9e9dff633c9949a88daefca6f42de1b64d Author: thuan.tran Date: Tue, 4 Feb 2020 16:50:59 +0700 mds: ignore flow control message on disabled FCTRL agent [#3148] - The confused error in mds log since disabled FCTRL agent receive a flow control message type from enabled FCTRL agent. MDS should ignore flow control message on disabled FCTRL agent. - Refactor mds_tipc_fctrl_drop_data() for code consistency. - Also print Adest info detail in logging for easy troubleshoot. Complete diffstat: -- src/mds/mds_dt_common.c| 65 +++--- src/mds/mds_tipc_fctrl_intf.cc | 63 +--- 2 files changed, 69 insertions(+), 59 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
