[PATCH 4/5] posix: add user-facing hooks for shm object management

Chris Johns chrisj at rtems.org
Wed Aug 24 02:00:35 UTC 2016


On 18/08/2016 6:02 AM, Gedare Bloom wrote:
> ---
>  cpukit/posix/Makefile.am                   |  2 +
>  cpukit/posix/include/rtems/posix/shm.h     | 65 ++++++++++++++++++++++++-----
>  cpukit/posix/include/rtems/posix/shmimpl.h | 11 +++++
>  cpukit/posix/src/shmheap.c                 | 60 ++++++++++++++++++++++++++
>  cpukit/posix/src/shmopen.c                 | 56 +++++--------------------
>  cpukit/posix/src/shmwkspace.c              | 67 ++++++++++++++++++++++++++++++
>  cpukit/sapi/include/confdefs.h             | 13 +++++-
>  testsuites/psxtests/Makefile.am            |  2 +-
>  testsuites/psxtests/configure.ac           |  1 +
>  9 files changed, 219 insertions(+), 58 deletions(-)
>  create mode 100644 cpukit/posix/src/shmheap.c
>  create mode 100644 cpukit/posix/src/shmwkspace.c
>
> diff --git a/cpukit/posix/Makefile.am b/cpukit/posix/Makefile.am
> index 5490401..1851535 100644
> --- a/cpukit/posix/Makefile.am
> +++ b/cpukit/posix/Makefile.am
> @@ -103,8 +103,10 @@ libposix_a_SOURCES += src/munlock.c
>  libposix_a_SOURCES += src/munmap.c
>  libposix_a_SOURCES += src/posix_madvise.c
>  libposix_a_SOURCES += src/shm.c
> +libposix_a_SOURCES += src/shmheap.c
>  libposix_a_SOURCES += src/shmopen.c
>  libposix_a_SOURCES += src/shmunlink.c
> +libposix_a_SOURCES += src/shmwkspace.c
>
>  ## MESSAGE_QUEUE_C_FILES
>  libposix_a_SOURCES += src/mqueue.c src/mqueueclose.c \
> diff --git a/cpukit/posix/include/rtems/posix/shm.h b/cpukit/posix/include/rtems/posix/shm.h
> index f3d7a91..eae0dcf 100644
> --- a/cpukit/posix/include/rtems/posix/shm.h
> +++ b/cpukit/posix/include/rtems/posix/shm.h
> @@ -30,6 +30,13 @@ extern "C" {
>   * Internal implementation support for POSIX shared memory.
>   * @{
>   */
> +typedef struct POSIX_Shm_Object_operations POSIX_Shm_Object_operations;
> +extern const POSIX_Shm_Object_operations _POSIX_Shm_Object_operations;
> +typedef struct {
> +  void                               *handle;
> +  size_t                              size;
> +  const POSIX_Shm_Object_operations  *ops;
> +} POSIX_Shm_Object;
>
>  typedef struct {
>     Objects_Control      Object;
> @@ -37,15 +44,7 @@ typedef struct {
>
>     int                  reference_count;
>
> -   void                *shm_object;
> -   size_t               shm_object_size;
> -   void                *( *shm_object_allocate )( size_t size );
> -   void                *( *shm_object_reallocate )(
> -                          void *ptr,
> -                          size_t curr_size,
> -                          size_t new_size
> -                        );
> -   void                 ( *shm_object_free ) ( void *ptr );
> +   POSIX_Shm_Object     shm_object;
>
>     uid_t                uid;
>     gid_t                gid;
> @@ -54,9 +53,55 @@ typedef struct {
>     time_t               atime;
>     time_t               mtime;
>     time_t               ctime;
> -
>  } POSIX_Shm_Control;
>

It is not clear to me what is being returned as an 'int' with these 
functions?

> +struct POSIX_Shm_Object_operations {
> +  int   ( *object_create ) ( POSIX_Shm_Control *shm, size_t size );
> +  int   ( *object_resize ) ( POSIX_Shm_Control *shm, size_t size );
> +  int   ( *object_delete ) ( POSIX_Shm_Control *shm );
> +  void *( *object_mmap   ) ( POSIX_Shm_Control *shm, size_t len, int prot, int flags, off_t off );
> +};
> +
> +extern int _POSIX_Shm_Object_create_from_workspace(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +);
> +
> +extern int _POSIX_Shm_Object_delete_from_workspace( POSIX_Shm_Control *shm );
> +
> +extern int _POSIX_Shm_Object_resize_from_workspace(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +);
> +
> +extern void *_POSIX_Shm_Object_mmap_from_workspace(
> +  POSIX_Shm_Control *shm,
> +  size_t len,
> +  int prot,
> +  int flags,
> +  off_t off
> +);
> +
> +extern int _POSIX_Shm_Object_create_from_heap(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +);
> +
> +extern int _POSIX_Shm_Object_delete_from_heap( POSIX_Shm_Control *shm );
> +
> +extern int _POSIX_Shm_Object_resize_from_heap(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +);
> +
> +extern void *_POSIX_Shm_Object_mmap_from_heap(
> +  POSIX_Shm_Control *shm,
> +  size_t len,
> +  int prot,
> +  int flags,
> +  off_t off
> +);
> +
>  /** @} */
>
>  #ifdef __cplusplus
> diff --git a/cpukit/posix/include/rtems/posix/shmimpl.h b/cpukit/posix/include/rtems/posix/shmimpl.h
> index 2df4bd3..c012771 100644
> --- a/cpukit/posix/include/rtems/posix/shmimpl.h
> +++ b/cpukit/posix/include/rtems/posix/shmimpl.h
> @@ -81,6 +81,17 @@ RTEMS_INLINE_ROUTINE POSIX_Shm_Control *_POSIX_Shm_Get_by_name(
>      error
>    );
>  }
> +
> +RTEMS_INLINE_ROUTINE void _POSIX_Shm_Update_mtime_ctime(
> +  POSIX_Shm_Control *shm
> +)
> +{
> +  struct timeval now;
> +  gettimeofday( &now, 0 );
> +  shm->mtime = now.tv_sec;
> +  shm->ctime = now.tv_sec;
> +}
> +
>  /** @} */
>
>  #ifdef __cplusplus
> diff --git a/cpukit/posix/src/shmheap.c b/cpukit/posix/src/shmheap.c
> new file mode 100644
> index 0000000..4f6f105
> --- /dev/null
> +++ b/cpukit/posix/src/shmheap.c
> @@ -0,0 +1,60 @@
> +/**
> + * @file
> + */
> +
> +/*
> + * Copyright (c) 2016 Gedare Bloom.
> + *
> + * The license and distribution terms for this file may be
> + * found in the file LICENSE in this distribution or at
> + * http://www.rtems.org/license/LICENSE.
> + */
> +
> +#if HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <rtems/posix/shmimpl.h>
> +
> +int _POSIX_Shm_Object_create_from_heap(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +)
> +{
> +  shm->shm_object.handle = malloc( size );
> +  shm->shm_object.size = size;
> +  return 0;

Looking further down I see this is an error number of some form. Why not 
test the result of malloc rather than test the handle?

> +}
> +
> +int _POSIX_Shm_Object_delete_from_heap( POSIX_Shm_Control *shm )
> +{
> +  free( shm->shm_object.handle );
> +  shm->shm_object.handle = NULL;
> +  shm->shm_object.size = 0;
> +  return 0;
> +}
> +
> +int _POSIX_Shm_Object_resize_from_heap(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +)
> +{
> +  shm->shm_object.handle = realloc( shm->shm_object.handle, size );
> +  shm->shm_object.size = size;
> +  return 0;

realloc can fail.

> +}
> +
> +void *_POSIX_Shm_Object_mmap_from_heap(
> +  POSIX_Shm_Control *shm,
> +  size_t len,
> +  int prot,
> +  int flags,
> +  off_t off
> +)
> +{
> +  return NULL;
> +}
> +
> +
> diff --git a/cpukit/posix/src/shmopen.c b/cpukit/posix/src/shmopen.c
> index c8284e6..7823785 100644
> --- a/cpukit/posix/src/shmopen.c
> +++ b/cpukit/posix/src/shmopen.c
> @@ -28,55 +28,21 @@
>
>  static const rtems_filesystem_file_handlers_r shm_handlers;
>
> -static void* shm_reallocate_from_workspace(
> -  void *ptr,
> -  size_t curr_size,
> -  size_t new_size
> -)
> -{
> -  char *new_ptr;
> -  char *old_ptr = (char*) ptr;
> -  int i;
> -
> -  if ( new_size == 0 ) {
> -    _Workspace_Free( ptr );
> -    return NULL;
> -  }
> -
> -  new_ptr = _Workspace_Allocate_or_fatal_error( new_size );
> -  /* memcpy ... */
> -  for ( i = 0; i < curr_size && i < new_size; i++ ) {
> -    new_ptr[i] = old_ptr[i];
> -  }
> -  _Workspace_Free(ptr);
> -
> -  return new_ptr;
> -}
> -
> -static inline void shm_mtime_ctime_update( POSIX_Shm_Control *shm )
> -{
> -  struct timeval now;
> -
> -  gettimeofday( &now, 0 );
> -  shm->mtime = now.tv_sec;
> -  shm->ctime = now.tv_sec;
> -}
> -
>  static int shm_ftruncate( rtems_libio_t *iop, off_t length )
>  {
>    Thread_queue_Context  queue_context;
>    POSIX_Shm_Control *shm = (POSIX_Shm_Control *) iop->data1;
> +  int err;
>
>    _POSIX_Shm_Acquire_critical( shm, &queue_context );
>
> -  shm->shm_object = (*shm->shm_object_reallocate)(
> -      shm->shm_object,
> -      shm->shm_object_size,
> -      length
> -  );
> -  shm->shm_object_size = length;
> +  err = (*shm->shm_object.ops->object_resize)( shm, length );

Is the handle tested or should the err value be set in the handlers?

It would be nice to have what expected clearly documented.

>
> -  shm_mtime_ctime_update( shm );
> +  if ( err != 0 ) {
> +    rtems_set_errno_and_return_minus_one( err );
> +  }
> +
> +  _POSIX_Shm_Update_mtime_ctime( shm );
>
>    _POSIX_Shm_Release( shm, &queue_context );
>    return 0;
> @@ -110,11 +76,9 @@ static inline POSIX_Shm_Control *shm_allocate(
>    gettimeofday( &tv, 0 );
>
>    shm->reference_count = 1;
> -  shm->shm_object = NULL;
> -  shm->shm_object_allocate = _Workspace_Allocate_or_fatal_error;
> -  shm->shm_object_reallocate = shm_reallocate_from_workspace;
> -  shm->shm_object_free = _Workspace_Free;
> -  shm->shm_object_size = 0;
> +  shm->shm_object.handle = NULL;
> +  shm->shm_object.size = 0;
> +  shm->shm_object.ops = &_POSIX_Shm_Object_operations;
>    shm->mode = mode & ~rtems_filesystem_umask;
>    shm->uid = geteuid();
>    shm->gid = getegid();
> diff --git a/cpukit/posix/src/shmwkspace.c b/cpukit/posix/src/shmwkspace.c
> new file mode 100644
> index 0000000..f4f79a1
> --- /dev/null
> +++ b/cpukit/posix/src/shmwkspace.c
> @@ -0,0 +1,67 @@
> +/**
> + * @file
> + */
> +
> +/*
> + * Copyright (c) 2016 Gedare Bloom.
> + *
> + * The license and distribution terms for this file may be
> + * found in the file LICENSE in this distribution or at
> + * http://www.rtems.org/license/LICENSE.
> + */
> +
> +#if HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <rtems/score/wkspace.h>
> +#include <rtems/posix/shmimpl.h>
> +
> +int _POSIX_Shm_Object_create_from_workspace(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +)
> +{
> +  shm->shm_object.handle = _Workspace_Allocate_or_fatal_error( size );
> +  shm->shm_object.size = size;
> +  return 0;
> +}
> +
> +int _POSIX_Shm_Object_delete_from_workspace( POSIX_Shm_Control *shm )
> +{
> +  _Workspace_Free( shm->shm_object.handle );
> +  shm->shm_object.handle = NULL;
> +  shm->shm_object.size = 0;
> +  return 0;
> +}
> +
> +int _POSIX_Shm_Object_resize_from_workspace(
> +  POSIX_Shm_Control *shm,
> +  size_t size
> +)
> +{
> +  int err;
> +
> +  if ( size == 0 ) {
> +    err = _POSIX_Shm_Object_delete_from_workspace( shm );
> +  } else if ( shm->shm_object.handle == NULL && shm->shm_object.size == 0 ) {
> +    err = _POSIX_Shm_Object_create_from_workspace( shm, size );
> +  } else {
> +    err = EIO;
> +  }
> +  return err;
> +}

Ah ok some this one does set a return code but uses handle checks.

Chris

> +
> +void *_POSIX_Shm_Object_mmap_from_workspace(
> +  POSIX_Shm_Control *shm,
> +  size_t len,
> +  int prot,
> +  int flags,
> +  off_t off
> +)
> +{
> +  return NULL;
> +}
> +
> +
> diff --git a/cpukit/sapi/include/confdefs.h b/cpukit/sapi/include/confdefs.h
> index 385bf62..597c2f3 100644
> --- a/cpukit/sapi/include/confdefs.h
> +++ b/cpukit/sapi/include/confdefs.h
> @@ -2658,6 +2658,17 @@ extern rtems_initialization_tasks_table Initialization_tasks[];
>     */
>    #if !defined(CONFIGURE_MAXIMUM_POSIX_SHMS)
>      #define CONFIGURE_MAXIMUM_POSIX_SHMS 0
> +  #else
> +    #ifdef CONFIGURE_INIT
> +      #if !defined(CONFIGURE_HAS_OWN_POSIX_SHM_OBJECT_OPERATIONS)
> +        const POSIX_Shm_Object_operations _POSIX_Shm_Object_operations = {
> +          _POSIX_Shm_Object_create_from_workspace,
> +          _POSIX_Shm_Object_resize_from_workspace,
> +          _POSIX_Shm_Object_delete_from_workspace,
> +          _POSIX_Shm_Object_mmap_from_workspace
> +        };
> +      #endif
> +    #endif
>    #endif
>
>    /**
> @@ -2667,7 +2678,7 @@ extern rtems_initialization_tasks_table Initialization_tasks[];
>     * This is an internal parameter.
>     */
>    #define CONFIGURE_MEMORY_FOR_POSIX_SHMS(_shms) \
> -    _Configure_Object_RAM(_shms, sizeof(POSIX_Shm_Control) )
> +    _Configure_POSIX_Named_Object_RAM(_shms, sizeof(POSIX_Shm_Control) )
>
>
>    #ifdef CONFIGURE_POSIX_INIT_THREAD_TABLE
> diff --git a/testsuites/psxtests/Makefile.am b/testsuites/psxtests/Makefile.am
> index 206c450..bc76339 100644
> --- a/testsuites/psxtests/Makefile.am
> +++ b/testsuites/psxtests/Makefile.am
> @@ -9,7 +9,7 @@ _SUBDIRS += psxhdrs psx01 psx02 psx03 psx04 psx05 psx06 psx07 psx08 psx09 \
>      psxcancel psxcancel01 psxclassic01 psxcleanup psxcleanup01 \
>      psxconcurrency01 psxcond01 psxcond02 psxconfig01 psxenosys \
>      psxitimer psxmsgq01 psxmsgq02 psxmsgq03 psxmsgq04 \
> -    psxmutexattr01 psxobj01 psxrwlock01 psxsem01 psxshm01 \
> +    psxmutexattr01 psxobj01 psxrwlock01 psxsem01 psxshm01 psxshm02 \
>      psxsignal01 psxsignal02 psxsignal03 psxsignal04 psxsignal05 psxsignal06 \
>      psxspin01 psxspin02 psxsysconf \
>      psxtime psxtimer01 psxtimer02 psxualarm psxusleep psxfatal01 psxfatal02 \
> diff --git a/testsuites/psxtests/configure.ac b/testsuites/psxtests/configure.ac
> index e150fc0..fa730da 100644
> --- a/testsuites/psxtests/configure.ac
> +++ b/testsuites/psxtests/configure.ac
> @@ -191,6 +191,7 @@ psxrdwrv/Makefile
>  psxrwlock01/Makefile
>  psxsem01/Makefile
>  psxshm01/Makefile
> +psxshm02/Makefile
>  psxsignal01/Makefile
>  psxsignal02/Makefile
>  psxsignal03/Makefile
>


More information about the devel mailing list