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

Gedare Bloom gedare at rtems.org
Wed Mar 19 17:58:42 UTC 2014


Thanks, looks good to me.

On Wed, Mar 19, 2014 at 12:29 PM, Christian Mauderer
<Christian.Mauderer at embedded-brains.de> wrote:
> I have updated the documentation.
>
> Further I put all changes into one patch. The fileio test would not work
> after applying the posix key patch to the shell. Therefore I shouldn't
> have split it up in the first place.
>
> Am 19.03.2014 17:17, schrieb Christian Mauderer:
>> 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.
>>
>> Update documentation for the shell.
>>
>> Adapt samples/fileio:
>> - Add necessary objects.
>> - Add login function and custom device name for better testing of the shell.
>> ---
>>  cpukit/libmisc/shell/shell.c       | 56 ++++++++++++++++++++++++++------------
>>  cpukit/libmisc/shell/shell.h       |  4 +--
>>  doc/shell/confinit.t               |  3 ++
>>  testsuites/samples/fileio/init.c   |  5 ++--
>>  testsuites/samples/fileio/system.h |  3 ++
>>  5 files changed, 49 insertions(+), 22 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);
>> +}
>> +
>> +/*
>> + *  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);
>> +  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
>> diff --git a/doc/shell/confinit.t b/doc/shell/confinit.t
>> index 6c8a23b..eb85fe1 100644
>> --- a/doc/shell/confinit.t
>> +++ b/doc/shell/confinit.t
>> @@ -252,3 +252,6 @@ This method invokes the @code{rtems_task_create} and @code{rtems_task_start}
>>  directives and as such may return any status code that those directives
>>  may return.
>>
>> +There is one POSIX key necessary for all shell instances together and one POSIX
>> +key value pair per instance. You should make sure that your RTEMS configuration
>> +accounts for these resources.
>> diff --git a/testsuites/samples/fileio/init.c b/testsuites/samples/fileio/init.c
>> index 80da4ab..2a0f9db 100644
>> --- a/testsuites/samples/fileio/init.c
>> +++ b/testsuites/samples/fileio/init.c
>> @@ -708,10 +708,11 @@ static void fileio_start_shell(void)
>>      "SHLL",                          /* task_name */
>>      RTEMS_MINIMUM_STACK_SIZE * 4,    /* task_stacksize */
>>      100,                             /* task_priority */
>> -    "/dev/console",                  /* devname */
>> +    "/dev/foobar",                   /* devname */
>> +    /* device is currently ignored by the shell if it is not a pty */
>>      false,                           /* forever */
>>      true,                            /* wait */
>> -    NULL                             /* login */
>> +    rtems_shell_login_check          /* login */
>>    );
>>  }
>>  #endif /* USE_SHELL */
>> diff --git a/testsuites/samples/fileio/system.h b/testsuites/samples/fileio/system.h
>> index 726a507..654f727 100644
>> --- a/testsuites/samples/fileio/system.h
>> +++ b/testsuites/samples/fileio/system.h
>> @@ -47,6 +47,9 @@ rtems_task Init(
>>  #define CONFIGURE_ATA_DRIVER_TASK_PRIORITY  14
>>  #endif
>>
>> +#define CONFIGURE_MAXIMUM_POSIX_KEYS             1
>> +#define CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS  1
>> +
>>  #if FILEIO_BUILD
>>    #define CONFIGURE_APPLICATION_NEEDS_LIBBLOCK
>>    #define CONFIGURE_BDBUF_MAX_READ_AHEAD_BLOCKS  2
>>
>
> --
> --------------------------------------------
> embedded brains GmbH
> Christian Mauderer     Dornierstr. 4
> D-82178 Puchheim       Germany
> email: Christian.Mauderer at embedded-brains.de
> Phone: +49-89-18 94 741-18
> Fax:   +49-89-18 94 741-08
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
> _______________________________________________
> rtems-devel mailing list
> rtems-devel at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-devel




More information about the devel mailing list