Hi AndersBj, will add CcbErrorString and additional comments while pushing.
/Neel. On Tuesday 24 February 2015 10:08 PM, Anders Björnerstedt wrote: > Ack from me with some minor comments. > > Tested with 0PBE, 1PBE, 2PBE. > > To be really super user friendly we could add an error string also, > ImmModel::setCcbErrorString(...) > as shown below. For this particular kind of error I think an error > string up front makes sense, > since the typical user that is hit by this problem is a novice or a > tester trying to set up a system. > > Minor comments inlined. > > On 02/19/2015 12:32 PM, [email protected] wrote: >> osaf/services/saf/immsv/immnd/ImmModel.cc | 18 ++++++++++++++++-- >> osaf/services/saf/immsv/immnd/ImmModel.hh | 3 ++- >> osaf/services/saf/immsv/immnd/immnd.h | 10 ++++++++++ >> 3 files changed, 28 insertions(+), 3 deletions(-) >> >> >> ERR_BAD_OPERATION is returned when : >> 1. Enabling PBE when no PBE file has been configured >> 2. Enabling PBE when imm has not been built with --enable-imm-pbe >> >> diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc >> b/osaf/services/saf/immsv/immnd/ImmModel.cc >> --- a/osaf/services/saf/immsv/immnd/ImmModel.cc >> +++ b/osaf/services/saf/immsv/immnd/ImmModel.cc >> @@ -735,9 +735,10 @@ immModel_ccbObjectModify(IMMND_CB *cb, >> bool* hasLongDns) >> { >> std::string objectName; >> + bool pbeFile = (cb->mPbeFile != NULL)?true:false; > Above line can be simplified to just: > > bool pbeFile = (cb->mPbeFile != NULL); > >> SaAisErrorT err = ImmModel::instance(&cb->immModel)-> >> ccbObjectModify(req, implConn, implNodeId, continuationId, >> - pbeConn, pbeNodeId, objectName, hasLongDns); >> + pbeConn, pbeNodeId, objectName, hasLongDns, pbeFile); >> if(err == SA_AIS_OK && !objectName.empty()) { >> *objName = (char*) malloc((objectName.length() + 1) * >> sizeof(char)); >> @@ -7707,7 +7708,8 @@ ImmModel::ccbObjectModify(const ImmsvOmC >> SaUint32T* pbeConnPtr, >> unsigned int* pbeNodeIdPtr, >> std::string& objectName, >> - bool* hasLongDns) >> + bool* hasLongDns, >> + bool pbeFile) >> { >> TRACE_ENTER(); >> osafassert(hasLongDns); >> @@ -7955,6 +7957,18 @@ ImmModel::ccbObjectModify(const ImmsvOmC >> err = SA_AIS_ERR_BAD_OPERATION; >> break; //out of for-loop >> } >> + > Add a comment at the end of the line below like: /* ENABLE_PBE defined > in immnd.h */ >> + if(modifiedRim && !ENABLE_PBE) { >> + LOG_NO("ERR_BAD_OPERATION: imm has not been built with >> --enable-imm-pbe"); > setCcbErrorString(ccb, "ERR_BAD_OPERATION: imm > has not been built with --enable-imm-pbe"); >> + err = SA_AIS_ERR_BAD_OPERATION; >> + break; >> + } >> + >> + if(modifiedRim && !pbeFile) { >> + LOG_NO("ERR_BAD_OPERATION: PBE file is not configured"); > setCcbErrorString(ccb, "ERR_BAD_OPERATION: PBE file > is not configured "); >> + err = SA_AIS_ERR_BAD_OPERATION; >> + break; >> + } >> i4 = classInfo->mAttrMap.find(attrName); >> if(i4==classInfo->mAttrMap.end()) { >> diff --git a/osaf/services/saf/immsv/immnd/ImmModel.hh >> b/osaf/services/saf/immsv/immnd/ImmModel.hh >> --- a/osaf/services/saf/immsv/immnd/ImmModel.hh >> +++ b/osaf/services/saf/immsv/immnd/ImmModel.hh >> @@ -244,7 +244,8 @@ public: >> SaUint32T* pbeConn, >> unsigned int* pbeNodeId, >> std::string& objectName, >> - bool* hasLongDns); >> + bool* hasLongDns, >> + bool pbeFile); >> SaAisErrorT ccbObjectDelete( >> const >> ImmsvOmCcbObjectDelete* req, >> diff --git a/osaf/services/saf/immsv/immnd/immnd.h >> b/osaf/services/saf/immsv/immnd/immnd.h >> --- a/osaf/services/saf/immsv/immnd/immnd.h >> +++ b/osaf/services/saf/immsv/immnd/immnd.h >> @@ -32,4 +32,14 @@ >> #include "immnd_cb.h" >> #include "immnd_init.h" >> +#ifdef HAVE_CONFIG_H >> +#include <config.h> >> +#endif >> + >> +#ifdef HAVE_IMM_PBE >> +#define ENABLE_PBE 1 >> +#else >> +#define ENABLE_PBE 0 >> +#endif >> + >> #endif /* IMMND_H */ > ------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
