[PATCH 1/2] cpukit/shell: Replace task variables with posix keys.

[Gedare Bloom]

On Wed, Mar 19, 2014 at 11:15 AM, Christian Mauderer
<christian.mauderer at embedded-brains.de> wrote:
> From: Christian Mauderer <Christian.Mauderer at embedded-brains.de>
>
> Use posix keys for current shell environment instead of task variables. With
> this patch the shell needs one posix-key and one posix-key-value-pair
> configured.
> ---
>  cpukit/libmisc/shell/shell.c | 56 ++++++++++++++++++++++++++++++--------------
>  cpukit/libmisc/shell/shell.h |  4 ++--
>  2 files changed, 40 insertions(+), 20 deletions(-)
>
> diff --git a/cpukit/libmisc/shell/shell.c b/cpukit/libmisc/shell/shell.c
> index 90bb512..9fa56b6 100644
> --- a/cpukit/libmisc/shell/shell.c
> +++ b/cpukit/libmisc/shell/shell.c
> @@ -37,8 +37,10 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <pwd.h>
> +#include <pthread.h>
> +#include <assert.h>
>
> -rtems_shell_env_t rtems_global_shell_env = {
> +static rtems_shell_env_t rtems_global_shell_env = {
>    .magic         = rtems_build_name('S', 'E', 'N', 'V'),
>    .devname       = CONSOLE_DEVICE_NAME,
>    .taskname      = "SHGL",
> @@ -54,7 +56,8 @@ rtems_shell_env_t rtems_global_shell_env = {
>    .login_check   = NULL
>  };
>
> -rtems_shell_env_t *rtems_current_shell_env = &rtems_global_shell_env;
> +static pthread_once_t rtems_shell_current_env_once = PTHREAD_ONCE_INIT;
> +static pthread_key_t rtems_shell_current_env_key;
>
>  /*
>   *  Initialize the shell user/process environment information
> @@ -99,6 +102,24 @@ static void rtems_shell_env_free(
>  }
>
>  /*
> + *  Create the posix key.
> + */
> +static void rtems_shell_current_env_make_key(void)
> +{
> +  (void) pthread_key_create(&rtems_shell_current_env_key, rtems_shell_env_free);
Should the return value be checked for error condition?

> +}
> +
> +/*
> + *  Return the current shell environment
> + */
> +rtems_shell_env_t *rtems_shell_get_current_env(void)
> +{
> +  void *ptr = pthread_getspecific(rtems_shell_current_env_key);
> +  assert (ptr != NULL);
Under what conditions would this be NULL? I suspect the function can
be simplified to just return pthread_getspecific(), or even done in
the macro at shell.h

> +  return (rtems_shell_env_t *) ptr;
> +}
> +
> +/*
>   *  Get a line of user input with modest features
>   */
>  static int rtems_shell_line_editor(
> @@ -650,6 +671,7 @@ bool rtems_shell_main_loop(
>    rtems_shell_env_t *shell_env;
>    rtems_shell_cmd_t *shell_cmd;
>    rtems_status_code  sc;
> +  int                eno;
>    struct termios     term;
>    struct termios     previous_term;
>    char              *prompt = NULL;
> @@ -667,23 +689,21 @@ bool rtems_shell_main_loop(
>
>    rtems_shell_initialize_command_set();
>
> -  shell_env =
> -  rtems_current_shell_env = rtems_shell_init_env( shell_env_arg );
> -
> -  /*
> -   * @todo chrisj
> -   * Remove the use of task variables. Change to have a single
> -   * allocation per shell and then set into a notepad register
> -   * in the TCB. Provide a function to return the pointer.
> -   * Task variables are a virus to embedded systems software.
> -   */
> -  sc = rtems_task_variable_add(
> -    RTEMS_SELF,
> -    (void*)&rtems_current_shell_env,
> -    rtems_shell_env_free
> +  eno = pthread_once(
> +    &rtems_shell_current_env_once,
> +    rtems_shell_current_env_make_key
>    );
> -  if (sc != RTEMS_SUCCESSFUL) {
> -    rtems_error(sc,"rtems_task_variable_add(current_shell_env):");
> +  assert(eno == 0);
> +
> +  shell_env = rtems_shell_init_env(shell_env_arg);
> +  if (shell_env == NULL) {
> +    rtems_error(0, "rtems_shell_init_env");
> +    return false;
> +  }
> +
> +  eno = pthread_setspecific(rtems_shell_current_env_key, shell_env);
> +  if (eno != 0) {
> +    rtems_error(0, "pthread_setspecific(shell_current_env_key)");
>      return false;
>    }
>
> diff --git a/cpukit/libmisc/shell/shell.h b/cpukit/libmisc/shell/shell.h
> index 079ef66..af09ac3 100644
> --- a/cpukit/libmisc/shell/shell.h
> +++ b/cpukit/libmisc/shell/shell.h
> @@ -205,8 +205,8 @@ bool rtems_shell_main_loop(
>    rtems_shell_env_t *rtems_shell_env
>  );
>
> -extern rtems_shell_env_t  rtems_global_shell_env;
> -extern rtems_shell_env_t *rtems_current_shell_env;
> +rtems_shell_env_t *rtems_shell_get_current_env(void);
> +#define rtems_current_shell_env rtems_shell_get_current_env()
>
>  /*
>   * The types of file systems we can mount. We have them broken out
> --
> 1.8.4.5
>
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel