Ack on this reversion patch 1 of 2.
I assume it reverts enough of #969 to allow upgrade from 4.5 to this
changeset to be successfully tested
with protocol4.6 flag off. I have not tested that.
But this patch could be pushed now and ticket #1267 updated with the
changeset without closing #1267.
I am still reviewing the second patch (the new version of the actual
#969 enhancement).
I am having doubts about the solution of the embedded union in the
IMMSV_OI_CCB_UPCALL_RSP struct.
The alternative would be to add a new IMMSV_OCTET_STRING at the end of
CCB_UPCALL_RSP, preferrably with a name
like 'objName' instead of octetString.
This could in theory cost some extra bytes, but UPCALL_RSP is embedded
in the bigger IMMND_EVT union so I doubt that.
The advantage is less change in legacy code and that in the future it
would be simpler to actually really remove the
SaNameT type from these structs (the intention of #969).
That would be done at some point in the future (OpenSAF 5.0 perhaps)
where we consolidate and shift to a complete
new set of message types. An upgrade to that system would need to be
made in more than one step.
But it would allow the 5.0 code to be cleaned up from old legacy message
handling and the SaNameT type in the server,
which was the point of enhancement #969. Of course that would mean that
the upgrade to 5.0 would not be "baseline free".
But at some point we need to be able to remove support for very old and
obsolete message types.
/AndersBj
On 03/17/2015 01:42 PM, Hung Nguyen wrote:
> osaf/libs/agents/saf/imma/imma_oi_api.c | 19 +-----
> osaf/libs/agents/saf/imma/imma_proc.c | 16 +----
> osaf/libs/common/immsv/immsv_evt.c | 78
> +++++++++++++----------
> osaf/libs/common/immsv/include/immsv_evt_model.h | 2 +-
> osaf/services/saf/immsv/immnd/ImmModel.cc | 4 +-
> osaf/services/saf/immsv/immnd/immnd_evt.c | 13 +---
> 6 files changed, 50 insertions(+), 82 deletions(-)
>
>
> Revert the change on IMMSV_OI_CCB_UPCALL_RSP in #969
>
> 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
> @@ -3621,7 +3621,6 @@ SaAisErrorT saImmOiAugmentCcbInitialize(
> SaVersionT version = {'A', 2, 11};
> SaUint32T adminOwnerId = 0;
> SaUint32T ccbId = 0;
> - IMMSV_OCTET_STRING *objectName = NULL;
>
> TRACE_ENTER();
>
> @@ -3745,17 +3744,7 @@ SaAisErrorT saImmOiAugmentCcbInitialize(
> init_evt.info.immnd.info.ccbUpcallRsp.ccbId = ccbId;
> init_evt.info.immnd.info.ccbUpcallRsp.implId = cbi->implId;
> init_evt.info.immnd.info.ccbUpcallRsp.inv = cbi->inv;
> -
> - objectName = &init_evt.info.immnd.info.ccbUpcallRsp.name;
> - size_t length = osaf_extended_name_length(&cbi->name);
> - objectName->buf = (char*) malloc((length + 1) * sizeof(char));
> - if (!objectName->buf) {
> - rc = SA_AIS_ERR_NO_MEMORY;
> - goto done;
> - }
> - memcpy(objectName->buf, osaf_extended_name_borrow(&cbi->name),
> length);
> - objectName->buf[length] = '\0';
> - objectName->size = length + 1;
> + init_evt.info.immnd.info.ccbUpcallRsp.name = cbi->name;
>
> /* Note that we register using the new OM handle as client for
> the aug-ccb */
> rc = imma_evt_fake_evs(cb, &init_evt, &out_evt,
> cl_node->syncr_timeout,
> @@ -3881,12 +3870,6 @@ SaAisErrorT saImmOiAugmentCcbInitialize(
> out_evt = NULL;
> }
>
> - if (objectName && objectName->buf) {
> - free(objectName->buf);
> - objectName->buf = NULL;
> - objectName->size = 0;
> - }
> -
> lock_fail:
> TRACE_LEAVE();
> return rc;
> 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
> @@ -2618,17 +2618,7 @@ static bool imma_process_callback_info(I
> ccbObjDelRpl.info.immnd.info.ccbUpcallRsp.oi_client_hdl =
> callback->lcl_imm_hdl;
> ccbObjDelRpl.info.immnd.info.ccbUpcallRsp.ccbId = callback->ccbID;
> ccbObjDelRpl.info.immnd.info.ccbUpcallRsp.inv =
> callback->inv;
> -
> - IMMSV_OCTET_STRING *objectName =
> &ccbObjDelRpl.info.immnd.info.ccbUpcallRsp.name;
> - size_t length =
> osaf_extended_name_length(&callback->name);
> - objectName->buf = (char*) malloc((length + 1) *
> sizeof(char));
> - if (!objectName->buf) { /* Just log error and
> drop this reply. */
> - LOG_ER("Failed to allocate memory, drop this
> reply");
> - break;
> - }
> - memcpy(objectName->buf,
> osaf_extended_name_borrow(&callback->name), length);
> - objectName->buf[length] = '\0';
> - objectName->size = length + 1;
> +
> osaf_extended_name_lend(osaf_extended_name_borrow(&(callback->name)),
> &(ccbObjDelRpl.info.immnd.info.ccbUpcallRsp.name));
>
> osafassert(m_NCS_LOCK(&cb->cb_lock,
> NCS_LOCK_WRITE) == NCSCC_RC_SUCCESS);
> locked = true;
> @@ -2676,10 +2666,6 @@ static bool imma_process_callback_info(I
> /*Cant do anything but log error and drop
> this reply. */
> TRACE_3("CcbObjectDeleteCallback: send reply
> to IMMND failed");
> }
> -
> - free(objectName->buf);
> - objectName->buf = NULL;
> - objectName->size = 0;
> } else {
> /* callback->inv == 0 means PBE (CCB or PRTO) or
> applier upcall, no reply. */
> osafassert(cl_node->isPbe || cl_node->isApplier);
> diff --git a/osaf/libs/common/immsv/immsv_evt.c
> b/osaf/libs/common/immsv/immsv_evt.c
> --- a/osaf/libs/common/immsv/immsv_evt.c
> +++ b/osaf/libs/common/immsv/immsv_evt.c
> @@ -1991,27 +1991,14 @@ static uint32_t immsv_evt_enc_sublevels(
> */
> } else if ((i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_COMPLETED_RSP_2) ||
> (i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_MODIFY_RSP_2) ||
> - (i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP_2)) {
> + (i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP_2) ||
> + (i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2))
> + {
> IMMSV_OCTET_STRING *os =
> &(i_evt->info.immnd.info.ccbUpcallRsp.errorString);
> if(!immsv_evt_enc_inline_text(__LINE__, o_ub, os)) {
> return NCSCC_RC_OUT_OF_MEM;
> }
> - } else if (i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2) {
> - IMMSV_OCTET_STRING *os =
> &(i_evt->info.immnd.info.ccbUpcallRsp.errorString);
> - if(!immsv_evt_enc_inline_text(__LINE__, o_ub, os)) {
> - return NCSCC_RC_OUT_OF_MEM;
> - }
> - os = &(i_evt->info.immnd.info.ccbUpcallRsp.name);
> - if(!immsv_evt_enc_inline_text(__LINE__, o_ub, os)) {
> - return NCSCC_RC_OUT_OF_MEM;
> - }
> - } else if ((i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP) ||
> - (i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_OI_CCB_AUG_INIT)) {
> - IMMSV_OCTET_STRING *os =
> &(i_evt->info.immnd.info.ccbUpcallRsp.name);
> - if(!immsv_evt_enc_inline_text(__LINE__, o_ub, os)) {
> - return NCSCC_RC_OUT_OF_MEM;
> - }
> } else if (i_evt->info.immnd.type ==
> IMMND_EVT_A2ND_IMM_ADMINIT) {
> IMMSV_OCTET_STRING *os =
> &(i_evt->info.immnd.info.adminitReq.i.adminOwnerName.octetString);
> if (strlen(os->buf) >= IMMSV_MAX_ADMO_NAME_LENGTH) {
> @@ -2539,19 +2526,12 @@ static uint32_t immsv_evt_dec_sublevels(
> }
> } else if ((o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_COMPLETED_RSP_2) ||
> (o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_MODIFY_RSP_2) ||
> - (o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP_2)) {
> + (o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP_2) ||
> + (o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2))
> + {
> IMMSV_OCTET_STRING *os =
> &(o_evt->info.immnd.info.ccbUpcallRsp.errorString);
> immsv_evt_dec_inline_string(i_ub, os);
> - } else if (o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2) {
> - IMMSV_OCTET_STRING *os =
> &(o_evt->info.immnd.info.ccbUpcallRsp.errorString);
> - immsv_evt_dec_inline_string(i_ub, os);
> - os = &(o_evt->info.immnd.info.ccbUpcallRsp.name);
> - immsv_evt_dec_inline_string(i_ub, os);
> - } else if ((o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP) ||
> - (o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_OI_CCB_AUG_INIT)) {
> - IMMSV_OCTET_STRING *os =
> &(o_evt->info.immnd.info.ccbUpcallRsp.name);
> - immsv_evt_dec_inline_string(i_ub, os);
> } else if (o_evt->info.immnd.type ==
> IMMND_EVT_A2ND_IMM_ADMINIT) {
> IMMSV_OCTET_STRING *os =
> &(o_evt->info.immnd.info.adminitReq.i.adminOwnerName.octetString);
> immsv_evt_dec_inline_string(i_ub, os);
> @@ -3681,13 +3661,27 @@ static uint32_t immsv_evt_enc_toplevel(I
> ncs_encode_32bit(&p8, immndevt->info.ccbUpcallRsp.result);
> ncs_enc_claim_space(o_ub, 4);
>
> + IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 2);
> + const char* value =
> osaf_extended_name_borrow(&immndevt->info.ccbUpcallRsp.name);
> + size_t length = strlen(value);
> + osafassert(length <= 0xffff);
> + ncs_encode_16bit(&p8, length);
> + ncs_enc_claim_space(o_ub, 2);
> +
> + /* name.value is top level because type is SaNameT */
> if((immndevt->type == IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP) ||
> (immndevt->type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2) ||
> (immndevt->type == IMMND_EVT_A2ND_OI_CCB_AUG_INIT)) {
> - IMMSV_RSRV_SPACE_ASSERT(p8, o_ub, 4);
> - ncs_encode_32bit(&p8,
> immndevt->info.ccbUpcallRsp.name.size);
> - ncs_enc_claim_space(o_ub, 4);
> - /* immndevt->info.ccbUpcallRsp.name.buf encoded by
> sublevel */
> + if (length) {
> + if(ncs_encode_n_octets_in_uba(o_ub,
> + (uint8_t*) value,
> + length)
> + != NCSCC_RC_SUCCESS) {
> + LOG_WA("Failure inside
> ncs_encode_n_octets_in_uba");
> + rc = NCSCC_RC_FAILURE;
> + break;
> + }
> + }
> }
>
> if((immndevt->type == IMMND_EVT_A2ND_CCB_COMPLETED_RSP_2) ||
> @@ -5072,13 +5066,29 @@ static uint32_t immsv_evt_dec_toplevel(N
> immndevt->info.ccbUpcallRsp.result = ncs_decode_32bit(&p8);
> ncs_dec_skip_space(i_ub, 4);
>
> + IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 2);
> + size_t length = ncs_decode_16bit(&p8);
> + ncs_dec_skip_space(i_ub, 2);
> +
> + osaf_extended_name_clear(&immndevt->info.ccbUpcallRsp.name);
> if((immndevt->type == IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP) ||
> (immndevt->type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2) ||
> (immndevt->type == IMMND_EVT_A2ND_OI_CCB_AUG_INIT)) {
> - IMMSV_FLTN_SPACE_ASSERT(p8, local_data, i_ub, 4);
> - immndevt->info.ccbUpcallRsp.name.size =
> ncs_decode_32bit(&p8);
> - ncs_dec_skip_space(i_ub, 4);
> - /* immndevt->info.ccbUpcallRsp.name.buf decoded by
> sublevel */
> + /* name.value is top level because type is SaNameT */
> + if (length) {
> + char* value = (char*) malloc(length + 1);
> + if (value == NULL ||
> ncs_decode_n_octets_from_uba(i_ub,
> + (uint8_t*) value,
> + length) !=
> + NCSCC_RC_SUCCESS) {
> + free(value);
> + LOG_WA("Failure inside
> ncs_decode_n_octets_from_uba");
> + rc = NCSCC_RC_FAILURE;
> + break;
> + }
> + value[length] = '\0';
> + osaf_extended_name_steal(value,
> &immndevt->info.ccbUpcallRsp.name);
> + }
> }
>
> if((immndevt->type == IMMND_EVT_A2ND_CCB_COMPLETED_RSP_2) ||
> diff --git a/osaf/libs/common/immsv/include/immsv_evt_model.h
> b/osaf/libs/common/immsv/include/immsv_evt_model.h
> --- a/osaf/libs/common/immsv/include/immsv_evt_model.h
> +++ b/osaf/libs/common/immsv/include/immsv_evt_model.h
> @@ -288,7 +288,7 @@ extern "C" {
> SaUint32T implId;
> SaUint32T inv;
> SaAisErrorT result;
> - IMMSV_OCTET_STRING name;
> + SaNameT name;
> IMMSV_OCTET_STRING errorString;
> } IMMSV_OI_CCB_UPCALL_RSP;
>
> 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
> @@ -5959,7 +5959,7 @@ ImmModel::ccbAugmentInit(immsv_oi_ccb_up
>
> TRACE_ENTER();
> /*Note: objectName is parent-name for the create case! */
> - std::string objectName(rsp->name.buf);
> + std::string objectName(osaf_extended_name_borrow(&rsp->name));
> osafassert(nameCheck(objectName)||nameToInternal(objectName));
>
> i = std::find_if(sCcbVector.begin(), sCcbVector.end(),
> CcbIdIs(ccbId));
> @@ -9611,7 +9611,7 @@ ImmModel::ccbObjDelContinuation(immsv_oi
> SaUint32T* reqConn, bool* augDelete)
> {
> TRACE_ENTER();
> - std::string objectName(rsp->name.buf);
> + std::string objectName(osaf_extended_name_borrow(&rsp->name));
>
> SaUint32T ccbId = rsp->ccbId;
> CcbInfo* ccb = 0;
> 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
> @@ -470,23 +470,12 @@ uint32_t immnd_evt_destroy(IMMSV_EVT *ev
> immsv_evt_free_name_list(evt->info.immnd.info.admReq.objectNames);
> evt->info.immnd.info.admReq.objectNames = NULL;
> } else if ((evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_CREATE_RSP_2) ||
> + (evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2) ||
> (evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_MODIFY_RSP_2) ||
> (evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_COMPLETED_RSP_2)) {
> free(evt->info.immnd.info.ccbUpcallRsp.errorString.buf);
> evt->info.immnd.info.ccbUpcallRsp.errorString.buf = NULL;
> evt->info.immnd.info.ccbUpcallRsp.errorString.size = 0;
> - } else if (evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_2) {
> - free(evt->info.immnd.info.ccbUpcallRsp.errorString.buf);
> - evt->info.immnd.info.ccbUpcallRsp.errorString.buf = NULL;
> - evt->info.immnd.info.ccbUpcallRsp.errorString.size = 0;
> - free(evt->info.immnd.info.ccbUpcallRsp.name.buf);
> - evt->info.immnd.info.ccbUpcallRsp.name.buf = NULL;
> - evt->info.immnd.info.ccbUpcallRsp.name.size = 0;
> - } else if ((evt->info.immnd.type ==
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP) ||
> - (evt->info.immnd.type == IMMND_EVT_A2ND_OI_CCB_AUG_INIT)) {
> - free(evt->info.immnd.info.ccbUpcallRsp.name.buf);
> - evt->info.immnd.info.ccbUpcallRsp.name.buf = NULL;
> - evt->info.immnd.info.ccbUpcallRsp.name.size = 0;
> } else if (evt->info.immnd.type == IMMND_EVT_A2ND_IMM_ADMINIT) {
> free(evt->info.immnd.info.adminitReq.i.adminOwnerName.octetString.buf);
> evt->info.immnd.info.adminitReq.i.adminOwnerName.octetString.buf = NULL;
>
------------------------------------------------------------------------------
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
