[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6083: sched/environ: Refine the environment variables storage layout

[GitBox <gi...@apache.org>]

pussuw commented on code in PR #6083:
URL: https://github.com/apache/incubator-nuttx/pull/6083#discussion_r851733651


##########
sched/environ/env_dup.c:
##########
@@ -82,40 +82,59 @@ int env_dup(FAR struct task_group_s *group)
     {
       /* Yes.. The parent task has an environment allocation. */
 
-      envlen = ptcb->group->tg_envsize;
-      envp   = NULL;
+      while (ptcb->group->tg_envp[envc] != NULL)
+        {
+          envc++;
+        }
 
       /* A special case is that the parent has an "empty" environment
        * allocation, i.e., there is an allocation in place but it
-       * contains no variable definitions and, hence, envlen == 0.
+       * contains no variable definitions and, hence, envc == 0.
        */
 
-      if (envlen > 0)
+      if (envc > 0)
         {
           /* There is an environment, duplicate it */
 
-          envp = (FAR char *)group_malloc(group, envlen + 1);
+          envp = group_malloc(group, sizeof(*envp) * (envc + 1));
           if (envp == NULL)
             {
               /* The parent's environment can not be inherited due to a
                * failure in the allocation of the child environment.
                */
 
-              envlen = 0;
-              ret    = -ENOMEM;
+              ret = -ENOMEM;
             }
           else
             {
+              envp[envc] = NULL;
+
               /* Duplicate the parent environment. */
 
-              memcpy(envp, ptcb->group->tg_envp, envlen + 1);
+              while (envc-- > 0)

Review Comment:
   Another option would be to allocate memory for the stings at the same time as you allocate memory for the string pointers.
   
   https://github.com/apache/incubator-nuttx/blob/master/binfmt/binfmt_copyargv.c does exactly this. I think it might be a bit less wasteful to allocate all of the memory at once ? Less chance for fragmentation too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org