[mpich-discuss] Hydra genv_fn() memory leaks fix

[Yauheni Zelenko]

Hi, Pavan!

Thank you for help!

I agree that structure may be convenient to add several environment variables at once. However memory management  is not clear: programmers should always keep in mind that created structure should be free after insertion to list.

Eugene.
________________________________________
From: Pavan Balaji [balaji at mcs.anl.gov]
Sent: Wednesday, August 04, 2010 11:34 AM
To: mpich-discuss at mcs.anl.gov
Cc: Yauheni Zelenko
Subject: Re: [mpich-discuss] Hydra genv_fn() memory leaks fix

Thanks. I've fixed a bunch of memory leaks in r7000
[https://trac.mcs.anl.gov/projects/mpich2/changeset/7000].

The env structure is a bit wasteful while reading the user parameters,
but it's convenient while passing around environment lists throughout
the code. But it shouldn't really affect performance.

  -- Pavan

On 08/02/2010 04:48 PM, Yauheni Zelenko wrote:
> Hi!
>
> I want to suggest fix for memory leaks in genv_fn(). Code is based on r6973. Please review code.
>
> I also wonder with adding environment to list is so complicated and involve several memory copies (strings and structures)? May be should just HYDU_append_env_to_list() use name and value arguments instead of intermediate environment structure?
>
> static HYD_status genv_fn(char *arg, char ***argv)
> {
>      char *env_name, *env_value, *str[2] = { NULL, NULL };
>      struct HYD_env *env;
>      HYD_status status = HYD_SUCCESS;
>
>      status = HYDU_strsplit(**argv,&str[0],&str[1], '=');
>      HYDU_ERR_POP(status, "string break returned error\n");
>      (*argv)++;
>
>      env_name = str[0];
>      if (str[1] == NULL) {       /* The environment is not of the form x=foo */
>          if (**argv == NULL) {
>              status = HYD_INTERNAL_ERROR;
>              goto fn_fail;
>          }
>          env_value = **argv;
>          (*argv)++;
>      }
>      else {
>          env_value = str[1];
>      }
>
>      status = HYDU_env_create(&env, env_name, env_value);
>      HYDU_ERR_POP(status, "unable to create env struct\n");
>
>      HYDU_append_env_to_list(*env,&HYD_handle.user_global.global_env.user);
>
>    fn_exit:
>
>      if (str[0])
>       HYDU_free(str[0]);
>      if (str[1])
>       HYDU_free(str[1]);
>      if (env)
>       HYDU_env_free(env);
>
>      return status;
>
>    fn_fail:
>      goto fn_exit;
> }
>
> Eugene.
> _______________________________________________
> mpich-discuss mailing list
> mpich-discuss at mcs.anl.gov
> https://lists.mcs.anl.gov/mailman/listinfo/mpich-discuss

--
Pavan Balaji
http://www.mcs.anl.gov/~balaji