Yes its OK, the only thought I had here was that perhaps there could be an advantage if the error string returned over the API was identical to the related syslog message.
/AndersBj -----Original Message----- From: Hans Feldt [mailto:hans.fe...@ericsson.com] Sent: den 7 april 2014 13:49 To: Zoran Milinkovic; reddy.neelaka...@oracle.com Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 2] IMM: add error strings for CCB operations for ERR_NOT_EXIST error [#459] Some feedback on the strings produced: instead of: "ERR_NOT_EXIST: object '%s' does not have an implementer and flag SA_IMM_CCB_REGISTERED_OI is set" we could have: "ERR_NOT_EXIST: object '%s' exist but no implementer (which is required)" and: "ERR_NOT_EXIST: object '%s' does not exist" Any comments? /Hans On 04/02/2014 03:36 PM, Zoran Milinkovic wrote: > osaf/services/saf/immsv/immnd/ImmModel.cc | 60 > +++++++++++++++++++++++++++++++ > osaf/services/saf/immsv/immnd/ImmModel.hh | 5 ++ > osaf/services/saf/immsv/immnd/immnd_evt.c | 27 ++++++++++++- > 3 files changed, 89 insertions(+), 3 deletions(-) > > > When an implementer is detached from an object/class, CCB operations return > ERR_NOT_EXIST error code. > The patch should give more information regarding the error code in the error > string. > > 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 > @@ -6950,6 +6950,10 @@ SaAisErrorT ImmModel::ccbObjectCreate(Im > TRACE_7("ERR_NOT_EXIST: object '%s' does not have an " > "implementer and flag SA_IMM_CCB_REGISTERED_OI is > set", > objectName.c_str()); > + setCcbErrorString(ccb, > + "ERR_NOT_EXIST: object '%s' does not have an " > + "implementer and flag SA_IMM_CCB_REGISTERED_OI > is set", > + objectName.c_str()); > err = SA_AIS_ERR_NOT_EXIST; > } > } else { /* SA_IMM_CCB_REGISTERED_OI NOT set */ @@ > -7790,6 +7794,10 @@ ImmModel::ccbObjectModify(const ImmsvOmC > TRACE_7("ERR_NOT_EXIST: object '%s' does not have an " > "implementer and flag SA_IMM_CCB_REGISTERED_OI is set", > objectName.c_str()); > + setCcbErrorString(ccb, > + "ERR_NOT_EXIST: object '%s' does not have an " > + "implementer and flag SA_IMM_CCB_REGISTERED_OI is > set", > + objectName.c_str()); > err = SA_AIS_ERR_NOT_EXIST; > } > } else { /* SA_IMM_CCB_REGISTERED_OI NOT set */ @@ -8201,6 > +8209,10 @@ ImmModel::deleteObject(ObjectMap::iterat > TRACE_7("ERR_NOT_EXIST: object '%s' does not have an > implementer " > "and flag SA_IMM_CCB_REGISTERED_OI is set", > oi->first.c_str()); > + setCcbErrorString(ccb, > + "ERR_NOT_EXIST: object '%s' does not have an > implementer " > + "and flag SA_IMM_CCB_REGISTERED_OI is set", > + oi->first.c_str()); > return SA_AIS_ERR_NOT_EXIST; > } > } else { /* SA_IMM_CCB_REGISTERED_OI NOT set */ @@ -8370,6 > +8382,54 @@ ImmModel::deleteObject(ObjectMap::iterat > return SA_AIS_OK; > } > > +void > +ImmModel::setCcbErrorString(CcbInfo *ccb, const char *errorString, > +...) { > + int errLen = strlen(errorString) + 1; > + char *fmtError = (char *)malloc(errLen); > + int len; > + va_list vl; > + > + va_start(vl, errorString); > + len = vsnprintf(fmtError, errLen, errorString, vl); > + va_end(vl); > + > + osafassert(len >= 0); > + len++; /* Reserve one byte for null-terminated sign '\0' */ > + if(len > errLen) { > + fmtError = (char *)realloc(fmtError, len); > + va_start(vl, errorString); > + osafassert(vsnprintf(fmtError, len, errorString, vl) >= 0); > + va_end(vl); > + } > + > + unsigned int ix=0; > + ImmsvAttrNameList* errStr = ccb->mErrorStrings; > + ImmsvAttrNameList** errStrTail = &(ccb->mErrorStrings); > + while(errStr) { > + if(!strncmp(fmtError, errStr->name.buf, len)) { > + TRACE_5("Discarding duplicate error string '%s' for ccb id %u", > + fmtError, ccb->mId); > + free(fmtError); > + return; > + } > + ++ix; > + errStrTail = &(errStr->next); > + errStr = errStr->next; > + } > + > + if(ix >= IMMSV_MAX_ATTRIBUTES) { > + TRACE_5("Discarding error string '%s' for ccb id %u (too many)", > + fmtError, ccb->mId); > + free(fmtError); > + } else { > + (*errStrTail) = (ImmsvAttrNameList *) > malloc(sizeof(ImmsvAttrNameList)); > + (*errStrTail)->next = NULL; > + (*errStrTail)->name.size = len; > + (*errStrTail)->name.buf = fmtError; > + } > +} > + > bool > ImmModel::hasLocalClassAppliers(ClassInfo* classInfo) > { > 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 > @@ -259,6 +259,11 @@ public: > IdVector& continuations, > unsigned int pbeIsLocal); > > + void setCcbErrorString( > + CcbInfo *ccb, > + const char *errorString, > + ...); > + > bool hasLocalClassAppliers(ClassInfo* classInfo); > bool hasLocalObjAppliers(const std::string& objName); > > diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c > b/osaf/services/saf/immsv/immnd/immnd_evt.c > --- a/osaf/services/saf/immsv/immnd/immnd_evt.c > +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c > @@ -5732,11 +5732,18 @@ static void immnd_evt_proc_object_create > memset(&send_evt, '\0', sizeof(IMMSV_EVT)); > send_evt.type = IMMSV_EVT_TYPE_IMMA; > send_evt.info.imma.info.errRsp.error = err; > - send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > + send_evt.info.imma.info.errRsp.errStrings = > +immModel_ccbGrabErrStrings(cb, evt->info.objCreate.ccbId); > + > + if(send_evt.info.imma.info.errRsp.errStrings) { > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; > + } else { > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > + } > > if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != > NCSCC_RC_SUCCESS) { > LOG_WA("Failed to send result to Agent over MDS"); > } > + > +immsv_evt_free_attrNames(send_evt.info.imma.info.errRsp.errStrings); > } > TRACE_LEAVE(); > } > @@ -5957,11 +5964,18 @@ static void immnd_evt_proc_object_modify > memset(&send_evt, '\0', sizeof(IMMSV_EVT)); > send_evt.type = IMMSV_EVT_TYPE_IMMA; > send_evt.info.imma.info.errRsp.error = err; > - send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > + send_evt.info.imma.info.errRsp.errStrings = > +immModel_ccbGrabErrStrings(cb, evt->info.objModify.ccbId); > + > + if(send_evt.info.imma.info.errRsp.errStrings) { > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; > + } else { > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > + } > > if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != > NCSCC_RC_SUCCESS) { > LOG_WA("Failed to send result to Agent over MDS"); > } > + > +immsv_evt_free_attrNames(send_evt.info.imma.info.errRsp.errStrings); > } > > done: > @@ -6715,11 +6729,18 @@ static void immnd_evt_proc_object_delete > memset(&send_evt, '\0', sizeof(IMMSV_EVT)); > send_evt.type = IMMSV_EVT_TYPE_IMMA; > send_evt.info.imma.info.errRsp.error = err; > - send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > + send_evt.info.imma.info.errRsp.errStrings = > +immModel_ccbGrabErrStrings(cb, evt->info.objDelete.ccbId); > + > + if(send_evt.info.imma.info.errRsp.errStrings) { > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR_2; > + } else { > + send_evt.info.imma.type = IMMA_EVT_ND2A_IMM_ERROR; > + } > > if (immnd_mds_send_rsp(cb, &(cl_node->tmpSinfo), &send_evt) != > NCSCC_RC_SUCCESS) { > LOG_WA("Failed to send result to Agent over MDS"); > } > + > +immsv_evt_free_attrNames(send_evt.info.imma.info.errRsp.errStrings); > } > } > > > ---------------------------------------------------------------------- > -------- _______________________________________________ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > ------------------------------------------------------------------------------ Put Bad Developers to Shame Dominate Development with Jenkins Continuous Integration Continuously Automate Build, Test & Deployment Start a new project now. Try Jenkins in the cloud. http://p.sf.net/sfu/13600_Cloudbees_APR _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Put Bad Developers to Shame Dominate Development with Jenkins Continuous Integration Continuously Automate Build, Test & Deployment Start a new project now. Try Jenkins in the cloud. http://p.sf.net/sfu/13600_Cloudbees_APR _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel