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

Christian Mauderer Christian.Mauderer at embedded-brains.de
Wed Mar 19 15:57:45 UTC 2014


Am 19.03.2014 16:31, schrieb 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?
I have the problem that there seems to be no easy possibility to
return a value from a function called with posix_once. Therefore I can
check the value but not do anything with it. But with the current
implementation of the POSIX keys in RTEMS the pthread_setspecific
fails if the key has not been created correctly.  Therefore every
error in this function would result in an error when calling
pthread_setspecific in the next step where the return value is
checked.

> 
>> +}
>> +
>> +/*
>> + *  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
pthread_getspecific returns NULL if there are no data associated with
the key or if the key has not been initialized. So in theory it should
never happen as long as everything is used right.

The only case for it to return NULL I can think of is if someone calls
the rtems_shell_get_current_env before the shell main loop has been
started. In this case it would give a hint on wrong usage of the
shell.

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

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



More information about the devel mailing list