Tentative ACK with same comment as Neel.
Plus a question on a small part of the code below, see inline.
Zoran Milinkovic wrote:
> osaf/libs/agents/saf/imma/imma_om_api.c | 24 ++++++++++++++++++------
> 1 files changed, 18 insertions(+), 6 deletions(-)
>
>
> Add missing check for the case that root name length is great than 0, and
> root name value length is 0.
> Fix validation of root name.
>
> diff --git a/osaf/libs/agents/saf/imma/imma_om_api.c
> b/osaf/libs/agents/saf/imma/imma_om_api.c
> --- a/osaf/libs/agents/saf/imma/imma_om_api.c
> +++ b/osaf/libs/agents/saf/imma/imma_om_api.c
> @@ -6335,13 +6335,25 @@ SaAisErrorT saImmOmSearchInitialize_2(Sa
> evt.info.immnd.type = IMMND_EVT_A2ND_SEARCHINIT;
> IMMSV_OM_SEARCH_INIT *req = &(evt.info.immnd.info.searchInit);
> req->client_hdl = immHandle;
> - if (rootName && !osaf_is_extended_name_empty(rootName) &&
> - osaf_is_extended_name_valid(rootName)) {
> - req->rootName.size = osaf_extended_name_length(rootName) + 1;
> +
> + int rootNameLength = 0;
> + if(rootName) {
>
Needs negation:
> + if(osaf_is_extended_name_valid(rootName)) {
> + rc = SA_AIS_ERR_INVALID_PARAM;
> + TRACE_3("ERR_INVALID_PARAM: root name is not valid");
> + goto bad_sync;
> + }
>
The code sequence below seems to do redundant computation of rootNameLength.
First it is obtained by calling osaf_extended_name_length() but then
there is (also) special
treatement of old names. That should not be necessary. According to
osafg_extend_name.h:
/**
* @brief Calculate the length of an SaNameT.
*
* This function returns the length of the SaNameT @a name. It may
abort the
* process if @a name is not valid, but the sanity checks in this
function are
* guaranteed to be no stricter than the checks in the
* osaf_is_extended_name_valid() function.
*
* This function works also with legacy SaNameT structures containing a
string
* that is not NUL-terminated, and strings where there is a mismatch
between
* the string length as indicated by the .length field an any NUL character
* contained in the string (in which case the shortest of the two string
* lengths is returned).
*/
size_t osaf_extended_name_length(const SaNameT* name);
> + rootNameLength = osaf_extended_name_length(rootName);
> + if(!osaf_is_an_extended_name(rootName)) {
> + rootNameLength =
> strnlen(osaf_extended_name_borrow(rootName), rootNameLength);
> + }
>
> + }
> + if (rootNameLength) {
> + req->rootName.size = rootNameLength + 1;
> req->rootName.buf = malloc(req->rootName.size); /* alloc-1 */
> memcpy(req->rootName.buf, osaf_extended_name_borrow(rootName),
> - (size_t) req->rootName.size - 1);
> - req->rootName.buf[req->rootName.size - 1] = 0;
> + (size_t) rootNameLength);
> + req->rootName.buf[rootNameLength] = 0;
> } else {
> req->rootName.size = 0;
> req->rootName.buf = NULL;
> @@ -6396,7 +6408,7 @@ SaAisErrorT saImmOmSearchInitialize_2(Sa
> }
> }
>
> - if (rootName && !osaf_is_extended_name_empty(rootName)) {
> + if (rootName && rootNameLength) {
> TRACE("root: %s param:%p", osaf_extended_name_borrow(rootName),
> searchParam);
> }
>
>
> ------------------------------------------------------------------------------
> Slashdot TV. Videos for Nerds. Stuff that Matters.
> http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
> _______________________________________________
> Opensaf-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
------------------------------------------------------------------------------
Slashdot TV. Videos for Nerds. Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
