Hi!
I ran "make cppcheck" and "make cpplint" on these new patches, and I
still get the following warnings for the two new files lgs_clm.cc and
lgs_clm.h:
[osaf/services/saf/logsv/lgs/lgs_clm.cc:98] ->
[osaf/services/saf/logsv/lgs/lgs_clm.cc:100]: (style) Variable 'rc' is
reassigned a value before the old one has been used.
./osaf/services/saf/logsv/lgs/lgs_clm.h:0: No #ifndef header guard
found, suggested CPP variable is:
OSAF_SERVICES_SAF_LOGSV_LGS_LGS_CLM_H_ [build/header_guard] [5]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:18: Include the directory when
naming .h files [build/include] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:19: Include the directory when
naming .h files [build/include] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:31: Lines should be <= 80
characters long [whitespace/line_length] [2]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:32: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:33: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:34: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:35: { should almost always be
at the end of the previous line [whitespace/braces] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:36: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:37: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:38: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:39: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:41: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:42: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:43: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:44: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:45: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:46: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:48: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:49: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:50: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:51: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:52: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:53: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:54: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:55: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:56: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:57: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:58: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:58: Missing space before {
[whitespace/braces] [5]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:59: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:60: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:61: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:62: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:63: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:64: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:65: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:66: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:67: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:68: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:69: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:70: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:71: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:72: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:72: Line ends in whitespace.
Consider deleting these extra spaces. [whitespace/end_of_line] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:73: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:74: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:75: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:76: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:77: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:78: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:80: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:81: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:85: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:86: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:86: Line ends in whitespace.
Consider deleting these extra spaces. [whitespace/end_of_line] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:95: { should almost always be
at the end of the previous line [whitespace/braces] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:96: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:97: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:97: Using C-style cast. Use
reinterpret_cast<lgs_cb_t *>(...) instead [readability/casting] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:98: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:99: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:100: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:101: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:102: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:103: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:104: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:105: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:106: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:107: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:108: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:109: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:110: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:111: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:111: Line ends in whitespace.
Consider deleting these extra spaces. [whitespace/end_of_line] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:112: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:112: Missing username in TODO;
it should look like "// TODO(my_username): Stuff." [readability/todo] [2]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:112: TODO(my_username) should
be followed by a space [whitespace/todo] [2]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:112: Should have a space
between // and comment [whitespace/comments] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:113: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:113: Lines should be <= 80
characters long [whitespace/line_length] [2]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:114: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:115: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:116: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:117: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:118: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:119: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:120: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:121: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:128: { should almost always be
at the end of the previous line [whitespace/braces] [4]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:129: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:130: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:131: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:133: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:134: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:136: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:137: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:138: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:139: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:140: Tab found; better to use
spaces [whitespace/tab] [1]
./osaf/services/saf/logsv/lgs/lgs_clm.cc:141: Tab found; better to use
spaces [whitespace/tab] [1]
regards,
Anders Widell
On 08/02/2016 10:17 AM, [email protected] wrote:
> Summary:lgsv: Log Service CLM integration [#1638] V2
> Review request for Trac Ticket(s): #1638
> Peer Reviewer(s): Vu, Lennart,Anders Widell
> Pull request to: <<LIST THE PERSON WITH PUSH ACCESS HERE>>
> Affected branch(es): default
> Development branch: defult
>
> --------------------------------
> Impacted area Impact y/n
> --------------------------------
> Docs n
> Build system n
> RPM/packaging n
> Configuration files n
> Startup scripts n
> SAF services n
> OpenSAF services y
> Core libraries n
> Samples n
> Tests n
> Other n
>
>
> Comments (indicate scope for each "y" above):
> ---------------------------------------------
>
> changeset 3dbef8972c5101149a4754c43557e72acda32833
> Author: A V Mahesh <[email protected]>
> Date: Tue, 02 Aug 2016 13:40:55 +0530
>
> lga: agent Cluster Membership (CLM) integration [#1638] V2
>
> V2 patch:
>
> Incorporated Anders Widell and Vu review comments , for more details
> see the review comments posted on V1 patch. Description:
>
> Form CLM integration is supported from Log Service A.02.02.
>
> At-least a A.02.02 LGA client will check CLM membership status of
> client's
> node. old LGA clients A.02.01 are always clm member.
>
> This patch enhanced the log service for Unavailability of the Log
> Service
> API on a Non-Member Node which will fail with SA_AIS_ERR_UNAVAILABLE.
>
> After this patch the Log Service does not provide service to processes
> on
> cluster nodes that are not in the cluster membership.
>
> If the node rejoins the cluster membership, processes executing on the
> node
> will be able to reinitialize new library handles and use the entire set
> of
> Log Service APIs that operate on these new handles; however, invocation
> of
> APIs that operate on handles acquired by any process before the node
> left
> the membership will continue to fail with SA_AIS_ERR_UNAVAILABLE (or
> with
> the special treatment described above for asynchronous calls) with the
> exception of saLogFinalize(), which is used to free the library handles
> and
> all resources associated with these handles. Hence, it is recommended
> for
> the processes to finalize the library handles as soon as the processes
> detect that the node left the membership.
>
> Detailed README will be provide soon.
>
> Following are expected Log Service API behavior :
>
> Case1: On Non-Member Node, Log Service API will fail with code
> SA_AIS_ERR_UNAVAILABLE (31) Case2: On Member Node after recovered from
> Non-
> Member Node, Log Service API will fail with code SA_AIS_ERR_UNAVAILABLE
> (31)
> Case3: Non-Member Node + (Headless) Log Service API will fail with code
> SA_AIS_ERR_UNAVAILABLE (31) Case4: On Non-Member Node + (Headless) +
> (Head
> Joined) Log Service API will fail with code SA_AIS_ERR_UNAVAILABLE (31)
>
> changeset 713685dbe1282e40293b06d3067235452c931e77
> Author: A V Mahesh <[email protected]>
> Date: Tue, 02 Aug 2016 13:42:17 +0530
>
> lgs: director Cluster Membership (CLM) integration [#1638] V2
>
> changeset c992f8bf5f5b2df528c50497e06556346c7fe6db
> Author: A V Mahesh <[email protected]>
> Date: Tue, 02 Aug 2016 13:42:48 +0530
>
> logtest: change related to Cluster Membership (CLM) integration [#1638]
> V2
> Test :
>
> #amf-adm lock safNode=PL-4,safCluster=myClmCluster
>
> <saLogStreamOpen_2> failed with code SA_AIS_ERR_UNAVAILABLE (31).
>
> #amf-adm unlock safNode=PL-4,safCluster=myClmCluster
>
> <saLogStreamOpen_2> failed with code SA_AIS_ERR_UNAVAILABLE (31).
>
> #amf-adm lock safNode=PL-4,safCluster=myClmCluster + (SC-1 & Sc-2
> /etc/init.d/opensafd stop)
>
> <saLogStreamOpen_2> failed with code SA_AIS_ERR_UNAVAILABLE (31)
>
> # amf-adm lock safNode=PL-4,safCluster=myClmCluster + ((SC-1 & Sc-2
> /etc/init.d/opensafd stop)) + ((SC-1 & Sc-2 /etc/init.d/opensafd
> start) + amf-adm unlock safNode=PL-4,safCluster=myClmCluster
>
> <saLogStreamOpen_2> failed with code SA_AIS_ERR_UNAVAILABLE (31)
>
> Expected but currently The Imm APIs are NOT working error -
> saImmOmInitialize FAILED: SA_AIS_ERR_LIBRARY (2)
>
>
> Complete diffstat:
> ------------------
> osaf/libs/agents/saf/lga/lga.h | 2 +
> osaf/libs/agents/saf/lga/lga_api.c | 47
> +++++++++++++++++++++++++++++++++++++++++++++
> osaf/libs/agents/saf/lga/lga_mds.c | 69
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> osaf/libs/agents/saf/lga/lga_util.c | 2 +
> osaf/libs/common/logsv/include/lgsv_defs.h | 6 ++++-
> osaf/libs/common/logsv/include/lgsv_msg.h | 17 ++++++++++++---
> osaf/services/saf/logsv/lgs/Makefile.am | 7 ++++-
> osaf/services/saf/logsv/lgs/lgs_cb.h | 15 ++++++++++++++
> osaf/services/saf/logsv/lgs/lgs_clm.cc | 142
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> osaf/services/saf/logsv/lgs/lgs_clm.h | 25 ++++++++++++++++++++++++
> osaf/services/saf/logsv/lgs/lgs_evt.cc | 143
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> osaf/services/saf/logsv/lgs/lgs_evt.h | 2 +
> osaf/services/saf/logsv/lgs/lgs_main.cc | 28
> +++++++++++++++++++++++++++
> osaf/services/saf/logsv/lgs/lgs_mds.cc | 38
> +++++++++++++++++++++++++++++++++++-
> osaf/services/saf/logsv/lgs/lgs_util.cc | 83
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/logsv/saflogtest.c | 2 +-
> 16 files changed, 619 insertions(+), 9 deletions(-)
>
>
> Testing Commands:
> -----------------
> <<LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES>>
>
>
> Testing, Expected Results:
> --------------------------
> <<PASTE COMMAND OUTPUTS / TEST RESULTS>>
>
>
> Conditions of Submission:
> -------------------------
> <<HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC>>
>
>
> Arch Built Started Linux distro
> -------------------------------------------
> mips n 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 ~/.hgrc file (i.e. username, 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
