Hi Neel,

I think we should check the length of mDir and mFile in immnd_initialize().
Checking for NULL is not enough.

Even when the pointer returned by getenv() is not null, the length still can be 
0.
That happens when the env variable is set like this:
export IMMSV_ROOT_DIRECTORY=""


Thanks,

Hung Nguyen - DEK Technologies


--------------------------------------------------------------------------------
From: Neelakanta Reddy reddy.neelaka...@oracle.com
Sent: Tuesday, June 21, 2016 12:06PM
To: Hung Nguyen, Zoran Milinkovic
     hung.d.ngu...@dektech.com.au, zoran.milinko...@ericsson.com
Cc: Opensaf-devel
     opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1 of 3] imm: Remove conditional statements using 'mDir' and 
'mFile' [#1881]


Hi Hung,

Reviewed and tested the patch.
Comments inline.

/Neel.

On 2016/06/17 10:04 AM, Hung Nguyen wrote:
>   osaf/services/saf/immsv/immnd/immnd_proc.c |  21 ++++++++-------------
>   1 files changed, 8 insertions(+), 13 deletions(-)
>
>
> 'mDir' and 'mFile' are checked during the initialization of IMMND.
> If they are NULL, IMMND will fail to start.
> We can assume that they are not NULL if IMMND starts successfully.
>
> diff --git a/osaf/services/saf/immsv/immnd/immnd_proc.c 
> b/osaf/services/saf/immsv/immnd/immnd_proc.c
> --- a/osaf/services/saf/immsv/immnd/immnd_proc.c
> +++ b/osaf/services/saf/immsv/immnd/immnd_proc.c
> @@ -413,14 +413,14 @@ uint32_t immnd_introduceMe(IMMND_CB *cb)
>               (cb->mPbeFile)?((cb->mRim == 
> SA_IMM_KEEP_REPOSITORY)?4:3):2;
>           TRACE("First immnd_introduceMe, sending pbeEnabled:%u WITH 
> params",
>               send_evt.info.immd.info.ctrl_msg.pbeEnabled);
> -        if(cb->mDir) {
Here check for len. if len is greater than "0" then do strdup.

> -            int len = strlen(cb->mDir);
> -            if( cb->mDir[len-1] == '/') {
> -                mdirDup = strndup((char *)cb->mDir, len-1);
> -            } else {
> -                mdirDup = strdup(cb->mDir);
> -            }
> +
> +        int len = strlen(cb->mDir);
> +        if (cb->mDir[len - 1] == '/') {
> +            mdirDup = strndup((char *) cb->mDir, len - 1);
> +        } else {
> +            mdirDup = strdup(cb->mDir);
>           }
> +
>           send_evt.info.immd.info.ctrl_msg.dir.size = strlen(mdirDup)+1;
>           send_evt.info.immd.info.ctrl_msg.dir.buf = (char *) mdirDup;
>           send_evt.info.immd.info.ctrl_msg.xmlFile.size = 
> strlen(cb->mFile)+1;
> @@ -1448,10 +1448,6 @@ static int immnd_forkLoader(IMMND_CB *cb
>       int i, j;
>         TRACE_ENTER();
> -    if (!cb->mDir && !cb->mFile) {
Here also, check for length for mDir and mFile.

/Neel.
> -        LOG_WA("No directory and no file-name=>IMM coming up empty");
> -        return (-1);
> -    }
>         if ((myAbsLen - myLen + loaderBaseLen) > 1023) {
>           LOG_ER("Pathname too long: %u max is 1023", myAbsLen - myLen 
> + loaderBaseLen);
> @@ -1474,8 +1470,7 @@ static int immnd_forkLoader(IMMND_CB *cb
>       }
>       if (pid == 0) {        /*Child */
>           /* TODO Should close file-descriptors ... */
> -        char *ldrArgs[5] = { loaderName, (char *)(cb->mDir ? cb->mDir 
> : ""),
> -                     (char *)(cb->mFile ? cb->mFile : "imm.xml"),
> +        char *ldrArgs[5] = { loaderName, (char *)cb->mDir, (char 
> *)cb->mFile,
>                        (preLoad)?"preload":0, 0
>           };


------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to