sched/environ: Refine the environment variables storage layout

to confirm the standard and then implement get_environ_ptr correctly

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
This commit is contained in:
Xiang Xiao 2022-04-16 19:38:17 +08:00 committed by Petro Karashchenko
parent 73aa337408
commit 2c82bef702
11 changed files with 138 additions and 164 deletions

View File

@ -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

View File

@ -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();

View File

@ -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);
if (group->tg_envp == NULL)
{
return -ENOENT;
}
/* Search for a name=value string with matching name */
end = &group->tg_envp[group->tg_envsize];
for (ptr = group->tg_envp;
ptr < end && !env_cmpname(pname, ptr);
ptr += (strlen(ptr) + 1));
for (i = 0; group->tg_envp[i] != NULL; i++)
{
if (env_cmpname(pname, group->tg_envp[i]))
{
return i;
}
}
/* Check for success */
return (ptr < end) ? ptr : NULL;
return -ENOENT;
}
#endif /* CONFIG_DISABLE_ENVIRON */

View File

@ -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.

View File

@ -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;
}

View File

@ -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 */

View File

@ -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;
}

View File

@ -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);
/* 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.
*/
group->tg_envsize -= len;
ret = OK;
group->tg_envp[index] = group->tg_envp[index + 1];
}
while (group->tg_envp[++index] != NULL);
return ret;
/* Free the old environment (if there was one) */
if (index == 1)
{
group_free(group, group->tg_envp);
group->tg_envp = NULL;
}
else
{
/* Reallocate the environment to reclaim a little memory */
group->tg_envp = group_realloc(group, group->tg_envp,
sizeof(*group->tg_envp) * (index + 1));
DEBUGASSERT(group->tg_envp != NULL);
}
}
#endif /* CONFIG_DISABLE_ENVIRON */

View File

@ -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:

View File

@ -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();

View File

@ -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