Hi Neelakanta, The new objectName variable is not used in the code. I would rather like to see adminOwnerName as some sort of string type than using SaNameT. This is not an issue in the patch, only the suggestion. The reason for this is to try to avoid using SaNameT type as much as possible, and replace it with SaImmAdminOwnerNameT (or other sting type).
Find other comments inline -----Original Message----- From: [email protected] [mailto:[email protected]] Sent: den 30 november 2016 06:39 To: Zoran Milinkovic <[email protected]>; Hung Duc Nguyen <[email protected]> Cc: [email protected] Subject: [PATCH 1 of 1] imm:allow augumentCcbInit with ROF as false in completed callback[#1956] V2 osaf/libs/agents/saf/imma/imma_cb.h | 5 + osaf/libs/agents/saf/imma/imma_db.c | 99 +++++++++++++++++++++++++++++++++ osaf/libs/agents/saf/imma/imma_oi_api.c | 60 +++++++------------ osaf/libs/agents/saf/imma/imma_proc.c | 64 ++------------------- 4 files changed, 133 insertions(+), 95 deletions(-) Two new variables objectName and adminOwnerName added to imma_oi_ccb_record. For CCB create operation adminOwnerName will be added. For CCB delete/modify operation objectName will be added diff --git a/osaf/libs/agents/saf/imma/imma_cb.h b/osaf/libs/agents/saf/imma/imma_cb.h --- a/osaf/libs/agents/saf/imma/imma_cb.h +++ b/osaf/libs/agents/saf/imma/imma_cb.h @@ -36,6 +36,10 @@ struct imma_oi_ccb_record { struct imma_callback_info* ccbCallback; SaImmHandleT privateAugOmHandle; SaImmAdminOwnerHandleT privateAoHandle; + SaNameT objectName;/* The Object name from the modif/delete ccb operationi. The objectName is used to obtain + adminOnerName when saImmOiAugmentCcbInitialize is called in completed-callback*/ [Zoran] objectName is not used in the code. It can be removed from the patch + SaNameT adminOwnerName; /* adminowner name of the ccb id, assigned at ccb-create-callback.The adminOwnerName is used + saImmOiAugmentCcbInitialize is called in completed-callback*/ }; typedef struct imma_client_node { @@ -197,6 +201,7 @@ int imma_oi_ccb_record_set_critical(IMMA int imma_oi_ccb_record_terminate(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId); int imma_oi_ccb_record_abort(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId); int imma_oi_ccb_record_exists(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId); +struct imma_oi_ccb_record * imma_oi_ccb_record_find(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId); int imma_oi_ccb_record_set_error(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId, const SaStringT errorString); SaStringT imma_oi_ccb_record_get_error(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId); void imma_oi_ccb_allow_error_string(IMMA_CLIENT_NODE *cl_node, SaImmOiCcbIdT ccbId); diff --git a/osaf/libs/agents/saf/imma/imma_db.c b/osaf/libs/agents/saf/imma/imma_db.c --- a/osaf/libs/agents/saf/imma/imma_db.c +++ b/osaf/libs/agents/saf/imma/imma_db.c @@ -24,6 +24,8 @@ *****************************************************************************/ #include "imma.h" +#include "osaf_extended_name.h" + /**************************************************************************** Name : imma_client_tree_init Description : This routine is used to initialize the client tree @@ -273,6 +275,9 @@ int imma_oi_ccb_record_delete(IMMA_CLIEN to_delete->mCcbErrorString = NULL; } + osaf_extended_name_free(&(to_delete->objectName)); + osaf_extended_name_free(&(to_delete->adminOwnerName)); + free(to_delete); TRACE_LEAVE(); return 1; @@ -541,6 +546,11 @@ int imma_oi_ccb_record_note_callback(IMM */ int rs = 0; + const SaImmAttrNameT admoNameAttr = SA_IMM_ATTR_ADMIN_OWNER_NAME; + SaImmAttrValuesT_2 **attr, **attributes = NULL; + SaImmAttrValuesT_2 *attrVal = NULL; + size_t attrDataSize = 0; + struct imma_oi_ccb_record *tmp = imma_oi_ccb_record_find(cl_node, ccbId); if(tmp && !(tmp->isAborted)) { @@ -553,7 +563,96 @@ int imma_oi_ccb_record_note_callback(IMM rs = 1; } } + if(callback){ + [Zoran] Remove extra lines + int noOfAttributes = 0; + + /* NOTE: The code below is practically a copy of the code + in immOm searchNext, for serving the attrValues structure. + This code should be factored out into some common function. + */ + IMMSV_ATTR_VALUES_LIST *p = callback->attrValues; + int i = 0; + while (p) { + ++noOfAttributes; + p = p->next; + } + + p = callback->attrValues; + + + if(cl_node->isImmA2fCbk) { + if(noOfAttributes) { + p = p->next; // Skip RDN. RDN is the first attribute + noOfAttributes--; + } + } + + attrDataSize = sizeof(SaImmAttrValuesT_2 *) * (noOfAttributes + 1); + attr = calloc(1, attrDataSize); /*alloc-1 */ + for (; i < noOfAttributes && p; i++, p = p->next) { + IMMSV_ATTR_VALUES *q = &(p->n); + attr[i] = calloc(1, sizeof(SaImmAttrValuesT_2)); /*alloc-2 */ + attr[i]->attrName = malloc(q->attrName.size + 1); /*alloc-3 */ + strncpy(attr[i]->attrName, (const char *)q->attrName.buf, q->attrName.size + 1); + attr[i]->attrName[q->attrName.size] = 0; /*redundant. */ + attr[i]->attrValuesNumber = q->attrValuesNumber; + attr[i]->attrValueType = (SaImmValueTypeT)q->attrValueType; + if (q->attrValuesNumber) { + attr[i]->attrValues = + calloc(q->attrValuesNumber, sizeof(SaImmAttrValueT)); /*alloc-4 */ + /*alloc-5 */ + attr[i]->attrValues[0] = + imma_copyAttrValue3(q->attrValueType, &(q->attrValue)); + + if (q->attrValuesNumber > 1) { + int ix; + IMMSV_EDU_ATTR_VAL_LIST *r = q->attrMoreValues; + for (ix = 1; ix < q->attrValuesNumber; ++ix) { + osafassert(r); + attr[i]->attrValues[ix] = /*alloc-5 */ + imma_copyAttrValue3(q->attrValueType, &(r->n)); + r = r->next; + }//for + }//if + }//if + }//for + attr[i] = NULL; /*redundant */ + + + /* Save a copy of the attrvals pointer in the callback to allow + saImmOiAugmentCcbInitialize to get access to the ccbCreateContext. + We cant use callback->attrValues because it is partially stolen + from (caused to be incomplete) in the loop above (see imma_copyAttrValue3). + */ + + callback->attrValsForCreateUc = (const SaImmAttrValuesT_2 **)attr; [Zoran] Move the above line in the IF statement for IMMA_CALLBACK_OI_CCB_CREATE. 'attrValsForCreateUc' is only used for CCB object create callback. + + if(callback->type == IMMA_CALLBACK_OI_CCB_CREATE && osaf_is_extended_name_empty(&(tmp->adminOwnerName))) { + attributes = (SaImmAttrValuesT_2 **) callback->attrValsForCreateUc; + int i=0; + while((attrVal = attributes[i++]) != NULL) { + if(strcmp(admoNameAttr, attrVal->attrName)==0) { + TRACE("Found %s attribute in object create upcall", admoNameAttr); + break; + } + } + + if(!attrVal || strcmp(attrVal->attrName, admoNameAttr) || (attrVal->attrValuesNumber!=1) || + (attrVal->attrValueType != SA_IMM_ATTR_SASTRINGT)) { + LOG_ER("imma_oi_ccb_record_note_callback:Missmatch on attribute %s", admoNameAttr); + abort(); + } + osaf_extended_name_alloc(*(SaStringT*) attrVal->attrValues[0], &(tmp->adminOwnerName)); + + } else if (((callback->type == IMMA_CALLBACK_OI_CCB_DELETE) || (callback->type == IMMA_CALLBACK_OI_CCB_MODIFY)) + && osaf_is_extended_name_empty(&(tmp->objectName))){ + SaConstStringT tmpStr = osaf_extended_name_borrow(&(callback->name)); + osaf_extended_name_alloc(tmpStr, &(tmp->objectName)); [Zoran] The whole IF block can be removed. 'objectName' is not used. + } + } + return rs; } diff --git a/osaf/libs/agents/saf/imma/imma_oi_api.c b/osaf/libs/agents/saf/imma/imma_oi_api.c --- a/osaf/libs/agents/saf/imma/imma_oi_api.c +++ b/osaf/libs/agents/saf/imma/imma_oi_api.c @@ -3565,46 +3565,25 @@ extern void immsv_om_handle_finalize( SaImmHandleT privateOmHandle) __attribute__((weak)); static SaAisErrorT -getAdmoName(SaImmHandleT privateOmHandle, IMMA_CALLBACK_INFO * cbi, SaNameT* admoNameOut) +getAdmoName(SaImmHandleT privateOmHandle, IMMA_CALLBACK_INFO * cbi, SaNameT* object, SaNameT* admoNameOut) [Zoran] 'object' is not used in the function. It can be removed from the function parameter list. { SaAisErrorT rc = SA_AIS_OK; const SaImmAttrNameT admoNameAttr = SA_IMM_ATTR_ADMIN_OWNER_NAME; - SaImmAttrValuesT_2 **attributes = NULL; - SaImmAttrValuesT_2 *attrVal = NULL; + TRACE_ENTER(); - if(cbi->type == IMMA_CALLBACK_OI_CCB_CREATE) { - /* Admo attribute for created object should be in attr list.*/ - int i=0; - attributes = (SaImmAttrValuesT_2 **) cbi->attrValsForCreateUc; - while((attrVal = attributes[i++]) != NULL) { - if(strcmp(admoNameAttr, attrVal->attrName)==0) { - TRACE("Found %s attribute in object create upcall", admoNameAttr); - break; - } - } - - if(!attrVal || strcmp(attrVal->attrName, admoNameAttr) || (attrVal->attrValuesNumber!=1) || - (attrVal->attrValueType != SA_IMM_ATTR_SASTRINGT)) { - LOG_ER("Missmatch on attribute %s", admoNameAttr); - abort(); - } - - osaf_extended_name_alloc(*(SaStringT*) attrVal->attrValues[0], admoNameOut); - - } else { - /* modify or delete => fetch admo attribute for object from server. */ - if((cbi->type != IMMA_CALLBACK_OI_CCB_DELETE) && - (cbi->type != IMMA_CALLBACK_OI_CCB_MODIFY)) { - LOG_ER("Inconsistency in callback type:%u", cbi->type); - abort(); - } - - osafassert(immsv_om_augment_ccb_get_admo_name); - rc = immsv_om_augment_ccb_get_admo_name(privateOmHandle, &(cbi->name), admoNameOut); - if(rc == SA_AIS_ERR_LIBRARY) { - LOG_ER("Missmatch on attribute %s for delete or modify", admoNameAttr); - } + /* modify or delete => fetch admo attribute for object from server. */ + if((cbi->type != IMMA_CALLBACK_OI_CCB_DELETE) && + (cbi->type != IMMA_CALLBACK_OI_CCB_MODIFY) && + (cbi->type != IMMA_CALLBACK_OI_CCB_COMPLETED)) { + LOG_ER("Inconsistency in callback type:%u", cbi->type); + abort(); + } + + osafassert(immsv_om_augment_ccb_get_admo_name); + rc = immsv_om_augment_ccb_get_admo_name(privateOmHandle, &(cbi->name), admoNameOut); + if(rc == SA_AIS_ERR_LIBRARY) { + LOG_ER("Missmatch on attribute %s for delete or modify", admoNameAttr); } /* attrVal found either in create callback, or fetched from server. */ @@ -3734,6 +3713,7 @@ SaAisErrorT saImmOiAugmentCcbInitialize( goto done; } + struct imma_oi_ccb_record *ccb_oi_record = imma_oi_ccb_record_find(cl_node, ccbId); cbi = imma_oi_ccb_record_ok_augment(cl_node, ccbId, &privateOmHandle, &privateAoHandle); if(!cbi) { TRACE_2("ERR_BAD_OPERATION: Ccb %u, is not in a state that " @@ -3848,11 +3828,17 @@ SaAisErrorT saImmOiAugmentCcbInitialize( if(!privateAoHandle) { TRACE("AugCcbinit: Admo has ReleaseOnFinalize FALSE " - "=> init separate admo => must fetch admo-name first"); + "=> init separate admo => must fetch admo-name first"); osafassert(locked == false); SaNameT admName;/* Used to get admo string name copied to stack.*/ - rc = getAdmoName(privateOmHandle, cbi, &admName); + if((cbi->type == IMMA_CALLBACK_OI_CCB_CREATE)|| ((cbi->type == IMMA_CALLBACK_OI_CCB_COMPLETED) && + !osaf_is_extended_name_empty(&(ccb_oi_record->adminOwnerName)))){ + admName = ccb_oi_record->adminOwnerName; [Zoran] Remove spaces in the line above and align the line with the correct indent Thanks, Zoran + } else { + rc = getAdmoName(privateOmHandle, cbi, &(ccb_oi_record->objectName),&admName); + } + if(rc != SA_AIS_OK) { TRACE("ERR_TRY_AGAIN: failed to obtain SaImmAttrAdminOwnerName %u", rc); if(rc != SA_AIS_ERR_TRY_AGAIN) { diff --git a/osaf/libs/agents/saf/imma/imma_proc.c b/osaf/libs/agents/saf/imma/imma_proc.c --- a/osaf/libs/agents/saf/imma/imma_proc.c +++ b/osaf/libs/agents/saf/imma/imma_proc.c @@ -2285,7 +2285,6 @@ static bool imma_process_callback_info(I IMMSV_EVT ccbObjCrRpl; bool locked = false; SaImmAttrValuesT_2 **attr = NULL; - size_t attrDataSize = 0; int i = 0; /* No need to check for o.iCallbkA2f.saImmOiCcbObjectCreateCallback * "o" is union and o.iCallbk.saImmOiCcbObjectCreateCallback is same @@ -2308,62 +2307,11 @@ static bool imma_process_callback_info(I const SaImmClassNameT className = callback->className; /*0 */ callback->className = NULL; - int noOfAttributes = 0; - - /* NOTE: The code below is practically a copy of the code - in immOm searchNext, for serving the attrValues structure. - This code should be factored out into some common function. - */ - IMMSV_ATTR_VALUES_LIST *p = callback->attrValues; - while (p) { - ++noOfAttributes; - p = p->next; - } - - p = callback->attrValues; - - if(cl_node->isImmA2fCbk) { - if(noOfAttributes) { - p = p->next; // Skip RDN. RDN is the first attribute - noOfAttributes--; - } else { - /* There must be at least one attribute value */ - localEr = SA_AIS_ERR_BAD_OPERATION; - clientCapable = false; - } - } - - if(localEr == SA_AIS_OK) { - attrDataSize = sizeof(SaImmAttrValuesT_2 *) * (noOfAttributes + 1); - attr = calloc(1, attrDataSize); /*alloc-1 */ - for (; i < noOfAttributes && p; i++, p = p->next) { - IMMSV_ATTR_VALUES *q = &(p->n); - attr[i] = calloc(1, sizeof(SaImmAttrValuesT_2)); /*alloc-2 */ - attr[i]->attrName = malloc(q->attrName.size + 1); /*alloc-3 */ - strncpy(attr[i]->attrName, (const char *)q->attrName.buf, q->attrName.size + 1); - attr[i]->attrName[q->attrName.size] = 0; /*redundant. */ - attr[i]->attrValuesNumber = q->attrValuesNumber; - attr[i]->attrValueType = (SaImmValueTypeT)q->attrValueType; - if (q->attrValuesNumber) { - attr[i]->attrValues = - calloc(q->attrValuesNumber, sizeof(SaImmAttrValueT)); /*alloc-4 */ - /*alloc-5 */ - attr[i]->attrValues[0] = - imma_copyAttrValue3(q->attrValueType, &(q->attrValue)); - - if (q->attrValuesNumber > 1) { - int ix; - IMMSV_EDU_ATTR_VAL_LIST *r = q->attrMoreValues; - for (ix = 1; ix < q->attrValuesNumber; ++ix) { - osafassert(r); - attr[i]->attrValues[ix] = /*alloc-5 */ - imma_copyAttrValue3(q->attrValueType, &(r->n)); - r = r->next; - }//for - }//if - }//if - }//for - attr[i] = NULL; /*redundant */ + + if(cl_node->isImmA2fCbk && !callback->attrValsForCreateUc) { + /* There must be at least one attribute value */ + localEr = SA_AIS_ERR_BAD_OPERATION; + clientCapable = false; } if(localEr == SA_AIS_OK && cl_node->isImmA2fCbk) { @@ -2388,7 +2336,6 @@ static bool imma_process_callback_info(I We cant use callback->attrValues because it is partially stolen from (caused to be incomplete) in the loop above (see imma_copyAttrValue3). */ - callback->attrValsForCreateUc = (const SaImmAttrValuesT_2 **)attr; if (localEr == SA_AIS_OK && !osaf_is_extended_names_enabled() @@ -2457,6 +2404,7 @@ static bool imma_process_callback_info(I free(className); /*free-0 */ free(objectName); /*free-6 */ + attr = (SaImmAttrValuesT_2 **) callback->attrValsForCreateUc; for (i = 0; attr[i]; ++i) { free(attr[i]->attrName); /*free-3 */ attr[i]->attrName = 0; ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
