You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by pk...@apache.org on 2022/04/18 07:33:42 UTC
[incubator-nuttx] branch master updated: sched/environ: Refine the environment variables storage layout
This is an automated email from the ASF dual-hosted git repository.
pkarashchenko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 2c82bef702 sched/environ: Refine the environment variables storage layout
2c82bef702 is described below
commit 2c82bef70225f1b8f4475aeaf5510079fe45f7be
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Sat Apr 16 19:38:17 2022 +0800
sched/environ: Refine the environment variables storage layout
to confirm the standard and then implement get_environ_ptr correctly
Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
---
include/nuttx/sched.h | 3 +--
sched/environ/env_dup.c | 45 ++++++++++++++++++++++---------
sched/environ/env_findvar.c | 27 +++++++++++--------
sched/environ/env_foreach.c | 13 ++++-----
sched/environ/env_getenv.c | 12 ++++-----
sched/environ/env_getenvironptr.c | 24 ++---------------
sched/environ/env_release.c | 12 ++++++---
sched/environ/env_removevar.c | 57 +++++++++++++++++++--------------------
sched/environ/env_setenv.c | 50 +++++++++++++++++++---------------
sched/environ/env_unsetenv.c | 45 +++----------------------------
sched/environ/environ.h | 10 +++----
11 files changed, 136 insertions(+), 162 deletions(-)
diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h
index 06235096eb..88659844f2 100644
--- a/include/nuttx/sched.h
+++ b/include/nuttx/sched.h
@@ -526,8 +526,7 @@ struct task_group_s
#ifndef CONFIG_DISABLE_ENVIRON
/* Environment variables **************************************************/
- size_t tg_envsize; /* Size of environment string allocation */
- FAR char *tg_envp; /* Allocated environment strings */
+ FAR char **tg_envp; /* Allocated environment strings */
#endif
#ifndef CONFIG_DISABLE_POSIX_TIMERS
diff --git a/sched/environ/env_dup.c b/sched/environ/env_dup.c
index c6bddf6a73..58f15bb9d5 100644
--- a/sched/environ/env_dup.c
+++ b/sched/environ/env_dup.c
@@ -64,8 +64,8 @@
int env_dup(FAR struct task_group_s *group)
{
FAR struct tcb_s *ptcb = this_task();
- FAR char *envp = NULL;
- size_t envlen;
+ FAR char **envp = NULL;
+ size_t envc = 0;
int ret = OK;
DEBUGASSERT(group != NULL && ptcb != NULL && ptcb->group != NULL);
@@ -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)
+ {
+ envp[envc] = group_malloc(group,
+ strlen(ptcb->group->tg_envp[envc]) + 1);
+ if (envp[envc] == NULL)
+ {
+ while (envp[++envc] != NULL)
+ {
+ group_free(group, envp[envc]);
+ }
+
+ group_free(group, envp);
+ ret = -ENOMEM;
+ break;
+ }
+
+ strcpy(envp[envc], ptcb->group->tg_envp[envc]);
+ }
}
}
- /* Save the size and child environment allocation. */
+ /* Save the child environment allocation. */
- group->tg_envsize = envlen;
- group->tg_envp = envp;
+ group->tg_envp = envp;
}
sched_unlock();
diff --git a/sched/environ/env_findvar.c b/sched/environ/env_findvar.c
index ce7df15d34..dfeb8efcad 100644
--- a/sched/environ/env_findvar.c
+++ b/sched/environ/env_findvar.c
@@ -75,7 +75,7 @@ static bool env_cmpname(const char *pszname, const char *peqname)
* pname - The variable name to find
*
* Returned Value:
- * A pointer to the name=value string in the environment
+ * A index to the name=value string in the environment
*
* Assumptions:
* - Not called from an interrupt handler
@@ -83,25 +83,30 @@ static bool env_cmpname(const char *pszname, const char *peqname)
*
****************************************************************************/
-FAR char *env_findvar(FAR struct task_group_s *group, FAR const char *pname)
+int env_findvar(FAR struct task_group_s *group, FAR const char *pname)
{
- FAR char *ptr;
- FAR char *end;
+ int i;
/* Verify input parameters */
DEBUGASSERT(group != NULL && pname != NULL);
- /* Search for a name=value string with matching name */
+ if (group->tg_envp == NULL)
+ {
+ return -ENOENT;
+ }
- end = &group->tg_envp[group->tg_envsize];
- for (ptr = group->tg_envp;
- ptr < end && !env_cmpname(pname, ptr);
- ptr += (strlen(ptr) + 1));
+ /* Search for a name=value string with matching name */
- /* Check for success */
+ for (i = 0; group->tg_envp[i] != NULL; i++)
+ {
+ if (env_cmpname(pname, group->tg_envp[i]))
+ {
+ return i;
+ }
+ }
- return (ptr < end) ? ptr : NULL;
+ return -ENOENT;
}
#endif /* CONFIG_DISABLE_ENVIRON */
diff --git a/sched/environ/env_foreach.c b/sched/environ/env_foreach.c
index 05a73a2e5a..176cb5084d 100644
--- a/sched/environ/env_foreach.c
+++ b/sched/environ/env_foreach.c
@@ -65,22 +65,23 @@ int env_foreach(FAR struct task_group_s *group,
env_foreach_t cb,
FAR void *arg)
{
- FAR char *ptr;
- FAR char *end;
int ret = OK;
+ int i;
/* Verify input parameters */
DEBUGASSERT(group != NULL && cb != NULL);
- /* Search for a name=value string with matching name */
+ if (group->tg_envp == NULL)
+ {
+ return ret;
+ }
- end = &group->tg_envp[group->tg_envsize];
- for (ptr = group->tg_envp; ptr < end; ptr += (strlen(ptr) + 1))
+ for (i = 0; group->tg_envp[i] != NULL; i++)
{
/* Perform the callback */
- ret = cb(arg, ptr);
+ ret = cb(arg, group->tg_envp[i]);
/* Terminate the traversal early if the callback so requests by
* returning a non-zero value.
diff --git a/sched/environ/env_getenv.c b/sched/environ/env_getenv.c
index c279cb5fa4..19bcca8228 100644
--- a/sched/environ/env_getenv.c
+++ b/sched/environ/env_getenv.c
@@ -60,7 +60,6 @@ FAR char *getenv(FAR const char *name)
{
FAR struct tcb_s *rtcb;
FAR struct task_group_s *group;
- FAR char *pvar;
FAR char *pvalue = NULL;
int ret = OK;
@@ -68,7 +67,7 @@ FAR char *getenv(FAR const char *name)
if (name == NULL)
{
- ret = EINVAL;
+ ret = -EINVAL;
goto errout;
}
@@ -80,20 +79,19 @@ FAR char *getenv(FAR const char *name)
/* Check if the variable exists */
- if (group == NULL || (pvar = env_findvar(group, name)) == NULL)
+ if (group == NULL || (ret = env_findvar(group, name)) < 0)
{
- ret = ENOENT;
goto errout_with_lock;
}
/* It does! Get the value sub-string from the name=value string */
- pvalue = strchr(pvar, '=');
+ pvalue = strchr(group->tg_envp[ret], '=');
if (pvalue == NULL)
{
/* The name=value string has no '=' This is a bug! */
- ret = EINVAL;
+ ret = -EINVAL;
goto errout_with_lock;
}
@@ -106,7 +104,7 @@ FAR char *getenv(FAR const char *name)
errout_with_lock:
sched_unlock();
errout:
- set_errno(ret);
+ set_errno(-ret);
return NULL;
}
diff --git a/sched/environ/env_getenvironptr.c b/sched/environ/env_getenvironptr.c
index 0ad5dc727e..acf633140d 100644
--- a/sched/environ/env_getenvironptr.c
+++ b/sched/environ/env_getenvironptr.c
@@ -54,29 +54,9 @@
FAR char **get_environ_ptr(void)
{
-#if 1
+ FAR struct tcb_s *tcb = this_task();
- /* Type of internal representation of environment is incompatible with
- * char ** return value.
- */
-
- return NULL;
-
-#else
-
- /* Return a reference to the thread-private environ in the TCB. */
-
- FAR struct tcb_s *ptcb = this_task();
- if (ptcb->envp)
- {
- return &ptcb->envp->ev_env;
- }
- else
- {
- return NULL;
- }
-
-#endif
+ return tcb->group->tg_envp;
}
#endif /* CONFIG_DISABLE_ENVIRON */
diff --git a/sched/environ/env_release.c b/sched/environ/env_release.c
index 412e2661a4..e9089a0819 100644
--- a/sched/environ/env_release.c
+++ b/sched/environ/env_release.c
@@ -61,12 +61,19 @@
void env_release(FAR struct task_group_s *group)
{
- DEBUGASSERT(group != NULL);
+ int i;
- /* Free any allocate environment strings */
+ DEBUGASSERT(group != NULL);
if (group->tg_envp)
{
+ /* Free any allocate environment strings */
+
+ for (i = 0; group->tg_envp[i] != NULL; i++)
+ {
+ group_free(group, group->tg_envp[i]);
+ }
+
/* Free the environment */
group_free(group, group->tg_envp);
@@ -76,7 +83,6 @@ void env_release(FAR struct task_group_s *group)
* task group structure are reset to initial values.
*/
- group->tg_envsize = 0;
group->tg_envp = NULL;
}
diff --git a/sched/environ/env_removevar.c b/sched/environ/env_removevar.c
index 04b7c87be7..f7afa192d1 100644
--- a/sched/environ/env_removevar.c
+++ b/sched/environ/env_removevar.c
@@ -30,6 +30,8 @@
#include <sched.h>
#include <assert.h>
+#include <nuttx/kmalloc.h>
+
#include "environ/environ.h"
/****************************************************************************
@@ -45,10 +47,10 @@
* Input Parameters:
* group - The task group with the environment containing the name=value
* pair
- * pvar - A pointer to the name=value pair in the restroom
+ * index - A index to the name=value pair in the restroom
*
* Returned Value:
- * Zero on success
+ * None
*
* Assumptions:
* - Not called from an interrupt handler
@@ -57,44 +59,39 @@
*
****************************************************************************/
-int env_removevar(FAR struct task_group_s *group, FAR char *pvar)
+void env_removevar(FAR struct task_group_s *group, int index)
{
- FAR char *end; /* Pointer to the end+1 of the environment */
- int alloc; /* Size of the allocated environment */
- int ret = ERROR;
+ DEBUGASSERT(group != NULL && index >= 0);
- DEBUGASSERT(group != NULL && pvar != NULL);
+ /* Free the allocate environment string */
- /* Verify that the pointer lies within the environment region */
+ group_free(group, group->tg_envp[index]);
- alloc = group->tg_envsize; /* Size of the allocated environment */
- end = &group->tg_envp[alloc]; /* Pointer to the end+1 of the environment */
+ /* Move all of the environment strings after the removed one 'down.'
+ * this is inefficient, but robably not high duty.
+ */
- if (pvar >= group->tg_envp && pvar < end)
+ do
{
- /* Set up for the removal */
-
- int len = strlen(pvar) + 1; /* Length of name=value string to remove */
- FAR char *src = &pvar[len]; /* Address of name=value string after */
- FAR char *dest = pvar; /* Location to move the next string */
- int count = end - src; /* Number of bytes to move (might be zero) */
-
- /* Move all of the environment strings after the removed one 'down.'
- * this is inefficient, but robably not high duty.
- */
-
- memmove(dest, src, count + 1);
+ group->tg_envp[index] = group->tg_envp[index + 1];
+ }
+ while (group->tg_envp[++index] != NULL);
- /* Then set to the new allocation size. The caller is expected to
- * call realloc at some point but we don't do that here because the
- * caller may add more stuff to the environment.
- */
+ /* Free the old environment (if there was one) */
- group->tg_envsize -= len;
- ret = OK;
+ if (index == 1)
+ {
+ group_free(group, group->tg_envp);
+ group->tg_envp = NULL;
}
+ else
+ {
+ /* Reallocate the environment to reclaim a little memory */
- return ret;
+ group->tg_envp = group_realloc(group, group->tg_envp,
+ sizeof(*group->tg_envp) * (index + 1));
+ DEBUGASSERT(group->tg_envp != NULL);
+ }
}
#endif /* CONFIG_DISABLE_ENVIRON */
diff --git a/sched/environ/env_setenv.c b/sched/environ/env_setenv.c
index 3dbfb80f41..979ab500c4 100644
--- a/sched/environ/env_setenv.c
+++ b/sched/environ/env_setenv.c
@@ -70,8 +70,8 @@ int setenv(FAR const char *name, FAR const char *value, int overwrite)
FAR struct tcb_s *rtcb;
FAR struct task_group_s *group;
FAR char *pvar;
- FAR char *newenvp;
- int newsize;
+ FAR char **envp;
+ int envc = 0;
int varlen;
int ret = OK;
@@ -114,7 +114,7 @@ int setenv(FAR const char *name, FAR const char *value, int overwrite)
/* Check if the variable already exists */
- if (group->tg_envp && (pvar = env_findvar(group, name)) != NULL)
+ if (group->tg_envp && (ret = env_findvar(group, name)) >= 0)
{
/* It does! Do we have permission to overwrite the existing value? */
@@ -131,7 +131,7 @@ int setenv(FAR const char *name, FAR const char *value, int overwrite)
* the environment buffer; this will happen below.
*/
- env_removevar(group, pvar);
+ env_removevar(group, ret);
}
/* Get the size of the new name=value string.
@@ -142,38 +142,44 @@ int setenv(FAR const char *name, FAR const char *value, int overwrite)
/* Then allocate or reallocate the environment buffer */
+ pvar = group_malloc(group, varlen);
+ if (pvar == NULL)
+ {
+ ret = ENOMEM;
+ goto errout_with_lock;
+ }
+
if (group->tg_envp)
{
- newsize = group->tg_envsize + varlen;
- newenvp = (FAR char *)group_realloc(group, group->tg_envp,
- newsize + 1);
- if (!newenvp)
+ while (group->tg_envp[envc] != NULL)
{
- ret = ENOMEM;
- goto errout_with_lock;
+ envc++;
}
- pvar = &newenvp[group->tg_envsize];
+ envp = group_realloc(group, group->tg_envp,
+ sizeof(*envp) * (envc + 2));
+ if (envp == NULL)
+ {
+ ret = ENOMEM;
+ goto errout_with_var;
+ }
}
else
{
- newsize = varlen;
- newenvp = (FAR char *)group_malloc(group, varlen + 1);
- if (!newenvp)
+ envp = group_malloc(group, sizeof(*envp) * 2);
+ if (envp == NULL)
{
ret = ENOMEM;
- goto errout_with_lock;
+ goto errout_with_var;
}
-
- pvar = newenvp;
}
- newenvp[newsize] = '\0';
+ envp[envc++] = pvar;
+ envp[envc] = NULL;
- /* Save the new buffer and size */
+ /* Save the new buffer */
- group->tg_envp = newenvp;
- group->tg_envsize = newsize;
+ group->tg_envp = envp;
/* Now, put the new name=value string into the environment buffer */
@@ -181,6 +187,8 @@ int setenv(FAR const char *name, FAR const char *value, int overwrite)
sched_unlock();
return OK;
+errout_with_var:
+ group_free(group, pvar);
errout_with_lock:
sched_unlock();
errout:
diff --git a/sched/environ/env_unsetenv.c b/sched/environ/env_unsetenv.c
index 2b6ba7e098..6d227d0d37 100644
--- a/sched/environ/env_unsetenv.c
+++ b/sched/environ/env_unsetenv.c
@@ -61,9 +61,6 @@ int unsetenv(FAR const char *name)
{
FAR struct tcb_s *rtcb = this_task();
FAR struct task_group_s *group = rtcb->group;
- FAR char *pvar;
- FAR char *newenvp;
- int newsize;
int ret = OK;
DEBUGASSERT(name && group);
@@ -71,48 +68,12 @@ int unsetenv(FAR const char *name)
/* Check if the variable exists */
sched_lock();
- if (group && (pvar = env_findvar(group, name)) != NULL)
+ if (group && (ret = env_findvar(group, name)) >= 0)
{
/* It does! Remove the name=value pair from the environment. */
- env_removevar(group, pvar);
-
- /* Reallocate the new environment buffer */
-
- newsize = group->tg_envsize;
- if (newsize <= 0)
- {
- /* Free the old environment (if there was one) */
-
- if (group->tg_envp != NULL)
- {
- group_free(group, group->tg_envp);
- group->tg_envp = NULL;
- }
-
- group->tg_envsize = 0;
- }
- else
- {
- /* Reallocate the environment to reclaim a little memory */
-
- newenvp = (FAR char *)group_realloc(group, group->tg_envp,
- newsize + 1);
-
- if (newenvp == NULL)
- {
- set_errno(ENOMEM);
- ret = ERROR;
- }
- else
- {
- /* Save the new environment pointer (it might have changed due
- * to reallocation).
- */
-
- group->tg_envp = newenvp;
- }
- }
+ env_removevar(group, ret);
+ ret = OK;
}
sched_unlock();
diff --git a/sched/environ/environ.h b/sched/environ/environ.h
index 3d8fdd012b..5787801f08 100644
--- a/sched/environ/environ.h
+++ b/sched/environ/environ.h
@@ -111,7 +111,7 @@ void env_release(FAR struct task_group_s *group);
* pname - The variable name to find
*
* Returned Value:
- * A pointer to the name=value string in the environment
+ * A index to the name=value string in the environment
*
* Assumptions:
* - Not called from an interrupt handler
@@ -119,7 +119,7 @@ void env_release(FAR struct task_group_s *group);
*
****************************************************************************/
-FAR char *env_findvar(FAR struct task_group_s *group, FAR const char *pname);
+int env_findvar(FAR struct task_group_s *group, FAR const char *pname);
/****************************************************************************
* Name: env_removevar
@@ -130,10 +130,10 @@ FAR char *env_findvar(FAR struct task_group_s *group, FAR const char *pname);
* Input Parameters:
* group - The task group with the environment containing the name=value
* pair
- * pvar - A pointer to the name=value pair in the restroom
+ * index - A index to the name=value pair in the restroom
*
* Returned Value:
- * Zero on success
+ * None
*
* Assumptions:
* - Not called from an interrupt handler
@@ -142,7 +142,7 @@ FAR char *env_findvar(FAR struct task_group_s *group, FAR const char *pname);
*
****************************************************************************/
-int env_removevar(FAR struct task_group_s *group, FAR char *pvar);
+void env_removevar(FAR struct task_group_s *group, int index);
#undef EXTERN
#ifdef __cplusplus