[PATCH 4/8] score: Improve _CORE_message_queue_Initialize()

Gedare Bloom gedare at rtems.org
Thu Sep 24 16:39:14 UTC 2020


On Thu, Sep 24, 2020 at 6:13 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> Return a status code and differentiate between error conditions.
>
> Update #4007.
> ---
>  cpukit/include/rtems/score/coremsgimpl.h | 29 ++++++++++++------------
>  cpukit/include/rtems/score/status.h      |  4 ++++
>  cpukit/posix/src/mqueueopen.c            | 17 +++++++-------
>  cpukit/rtems/src/msgqcreate.c            | 17 ++++++++------
>  cpukit/score/src/coremsg.c               | 10 ++++----
>  testsuites/sptests/sp77/init.c           |  2 +-
>  testsuites/sptests/spmsgq_err01/init.c   | 11 +++++----
>  7 files changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/cpukit/include/rtems/score/coremsgimpl.h b/cpukit/include/rtems/score/coremsgimpl.h
> index e598dce96a..9403fb95fc 100644
> --- a/cpukit/include/rtems/score/coremsgimpl.h
> +++ b/cpukit/include/rtems/score/coremsgimpl.h
> @@ -71,24 +71,25 @@ typedef int CORE_message_queue_Submit_types;
>  /**
>   * @brief Initializes a message queue.
>   *
> - * This package is the implementation of the CORE Message Queue Handler.
> - * This core object provides task synchronization and communication functions
> - * via messages passed to queue objects.
> + * @param[out] the_message_queue is the message queue to initialize.
> + *
> + * @param discipline is the blocking discipline for the message queue.
> + *
> + * @param maximum_pending_messages is the maximum number of messages that will
> + *   be allowed to be pending at any given time.
> + *
> + * @param maximum_message_size is the size of the largest message that may be
> + *   sent to this message queue instance.
>   *
> - * This routine initializes @a the_message_queue
> - *        based on the parameters passed.
> + * @retval STATUS_SUCCESSFUL The message queue was initialized.
>   *
> - * @param[out] the_message_queue The message queue to initialize.
> - * @param discipline The blocking discipline for the message queue.
> - * @param maximum_pending_messages The maximum number of messages
> - *        that will be allowed to pend at any given time.
> - * @param maximum_message_size The size of the largest message that
> - *        may be sent to this message queue instance.
> + * @retval STATUS_MESSAGE_QUEUE_INVALID_SIZE Calculations with the maximum
> + *   pending messages or maximum message size produced an integer overflow.
>   *
> - * @retval true The message queue can be initialized.
> - * @retval false Memory for the pending messages cannot be allocated.
> + * @retval STATUS_MESSAGE_QUEUE_NO_MEMORY There was not enough memory to
> + *   allocate the message buffers.
>   */
> -bool _CORE_message_queue_Initialize(
> +Status_Control _CORE_message_queue_Initialize(
>    CORE_message_queue_Control     *the_message_queue,
>    CORE_message_queue_Disciplines  discipline,
>    uint32_t                        maximum_pending_messages,
> diff --git a/cpukit/include/rtems/score/status.h b/cpukit/include/rtems/score/status.h
> index b257ccc5db..bc374a5467 100644
> --- a/cpukit/include/rtems/score/status.h
> +++ b/cpukit/include/rtems/score/status.h
> @@ -91,6 +91,10 @@ typedef enum {
>      STATUS_BUILD( STATUS_CLASSIC_INTERNAL_ERROR, EOVERFLOW ),
>    STATUS_MESSAGE_INVALID_SIZE =
>      STATUS_BUILD( STATUS_CLASSIC_INVALID_SIZE, EMSGSIZE ),
> +  STATUS_MESSAGE_QUEUE_INVALID_SIZE =
> +    STATUS_BUILD( STATUS_CLASSIC_INVALID_SIZE, ENOSPC ),
EINVAL maybe better?

@Joel: This is a bit outside the scope of the posix spec maybe, but
the intent seems to be to use EINVAL for an invalid size argument.

> +  STATUS_MESSAGE_QUEUE_NO_MEMORY =
> +    STATUS_BUILD( STATUS_CLASSIC_UNSATISFIED, ENOSPC ),
>    STATUS_MESSAGE_QUEUE_WAIT_IN_ISR =
>      STATUS_BUILD( STATUS_CLASSIC_INTERNAL_ERROR, EAGAIN ),
>    STATUS_MESSAGE_QUEUE_WAS_DELETED =
> diff --git a/cpukit/posix/src/mqueueopen.c b/cpukit/posix/src/mqueueopen.c
> index 35b8c923b1..af8abebea8 100644
> --- a/cpukit/posix/src/mqueueopen.c
> +++ b/cpukit/posix/src/mqueueopen.c
> @@ -60,6 +60,7 @@ static mqd_t _POSIX_Message_queue_Create(
>  {
>    POSIX_Message_queue_Control  *the_mq;
>    char                         *name;
> +  Status_Control                status;
>
>    /* length of name has already been validated */
>
> @@ -97,14 +98,14 @@ static mqd_t _POSIX_Message_queue_Create(
>     *  Joel: Cite POSIX or OpenGroup on above statement so we can determine
>     *        if it is a real requirement.
>     */
> -  if (
> -    !_CORE_message_queue_Initialize(
> -      &the_mq->Message_queue,
> -      CORE_MESSAGE_QUEUE_DISCIPLINES_FIFO,
> -      attr->mq_maxmsg,
> -      attr->mq_msgsize
> -    )
> -  ) {
> +  status = _CORE_message_queue_Initialize(
> +    &the_mq->Message_queue,
> +    CORE_MESSAGE_QUEUE_DISCIPLINES_FIFO,
> +    attr->mq_maxmsg,
> +    attr->mq_msgsize
> +  );
> +
> +  if ( status != STATUS_SUCCESSFUL ) {
>      _POSIX_Message_queue_Free( the_mq );
>      _Workspace_Free( name );
>      rtems_set_errno_and_return_value( ENOSPC, MQ_OPEN_FAILED );
translate the status in case we end up using something besides ENOSPC?

> diff --git a/cpukit/rtems/src/msgqcreate.c b/cpukit/rtems/src/msgqcreate.c
> index 3741347cc9..79b198199e 100644
> --- a/cpukit/rtems/src/msgqcreate.c
> +++ b/cpukit/rtems/src/msgqcreate.c
> @@ -41,6 +41,7 @@ rtems_status_code rtems_message_queue_create(
>  {
>    Message_queue_Control          *the_message_queue;
>    CORE_message_queue_Disciplines  discipline;
> +  Status_Control                  status;
>  #if defined(RTEMS_MULTIPROCESSING)
>    bool                            is_global;
>  #endif
> @@ -107,12 +108,14 @@ rtems_status_code rtems_message_queue_create(
>    else
>      discipline = CORE_MESSAGE_QUEUE_DISCIPLINES_FIFO;
>
> -  if ( ! _CORE_message_queue_Initialize(
> -           &the_message_queue->message_queue,
> -           discipline,
> -           count,
> -           max_message_size
> -         ) ) {
> +  status = _CORE_message_queue_Initialize(
> +    &the_message_queue->message_queue,
> +    discipline,
> +    count,
> +    max_message_size
> +  );
> +
> +  if ( status != STATUS_SUCCESSFUL ) {
>  #if defined(RTEMS_MULTIPROCESSING)
>      if ( is_global )
>          _Objects_MP_Close(
> @@ -121,7 +124,7 @@ rtems_status_code rtems_message_queue_create(
>
>      _Message_queue_Free( the_message_queue );
>      _Objects_Allocator_unlock();
> -    return RTEMS_UNSATISFIED;
> +    return STATUS_GET_CLASSIC( status );
>    }
>
>    _Objects_Open(
> diff --git a/cpukit/score/src/coremsg.c b/cpukit/score/src/coremsg.c
> index 57882f6426..450470396c 100644
> --- a/cpukit/score/src/coremsg.c
> +++ b/cpukit/score/src/coremsg.c
> @@ -26,7 +26,7 @@
>    ( SIZE_MAX - sizeof( uintptr_t ) - 1 \
>      - sizeof( CORE_message_queue_Buffer_control ) )
>
> -bool _CORE_message_queue_Initialize(
> +Status_Control _CORE_message_queue_Initialize(
>    CORE_message_queue_Control     *the_message_queue,
>    CORE_message_queue_Disciplines  discipline,
>    uint32_t                        maximum_pending_messages,
> @@ -37,7 +37,7 @@ bool _CORE_message_queue_Initialize(
>
>    /* Make sure the message size computation does not overflow */
>    if ( maximum_message_size > MESSAGE_SIZE_LIMIT ) {
> -    return false;
> +    return STATUS_MESSAGE_QUEUE_INVALID_SIZE;
>    }
>
>    buffer_size = RTEMS_ALIGN_UP( maximum_message_size, sizeof( uintptr_t ) );
> @@ -48,7 +48,7 @@ bool _CORE_message_queue_Initialize(
>
>    /* Make sure the memory allocation size computation does not overflow */
>    if ( maximum_pending_messages > SIZE_MAX / buffer_size ) {
> -    return false;
> +    return STATUS_MESSAGE_QUEUE_INVALID_SIZE;
>    }
>
>    the_message_queue->message_buffers = _Workspace_Allocate(
> @@ -56,7 +56,7 @@ bool _CORE_message_queue_Initialize(
>    );
>
>    if ( the_message_queue->message_buffers == NULL ) {
> -    return false;
> +    return STATUS_MESSAGE_QUEUE_NO_MEMORY;
>    }
>
>    the_message_queue->maximum_pending_messages   = maximum_pending_messages;
> @@ -80,5 +80,5 @@ bool _CORE_message_queue_Initialize(
>      buffer_size
>    );
>
> -  return true;
> +  return STATUS_SUCCESSFUL;
>  }
> diff --git a/testsuites/sptests/sp77/init.c b/testsuites/sptests/sp77/init.c
> index 2c4a71a73c..ada13643fa 100644
> --- a/testsuites/sptests/sp77/init.c
> +++ b/testsuites/sptests/sp77/init.c
> @@ -32,7 +32,7 @@ rtems_task Init(
>        &id
>    );
>
> -  fatal_directive_check_status_only(status , RTEMS_UNSATISFIED ,
> +  fatal_directive_check_status_only(status , RTEMS_INVALID_SIZE ,
>                                     "attempt to create message queue return: ");
>    TEST_END();
>
> diff --git a/testsuites/sptests/spmsgq_err01/init.c b/testsuites/sptests/spmsgq_err01/init.c
> index 1ff8490d1a..854377038e 100644
> --- a/testsuites/sptests/spmsgq_err01/init.c
> +++ b/testsuites/sptests/spmsgq_err01/init.c
> @@ -101,8 +101,9 @@ rtems_task Init(
>    /* not enough memory for messages */
>    status = rtems_message_queue_create(
>      Queue_name[ 1 ],
> -    INT_MAX,
> -    MESSAGE_SIZE,
> +    SIZE_MAX
> +      / ( sizeof( uintptr_t ) + sizeof( CORE_message_queue_Buffer_control ) ),
> +    1,
>      RTEMS_DEFAULT_ATTRIBUTES,
>      &Queue_id[ 1 ]
>    );
> @@ -123,10 +124,10 @@ rtems_task Init(
>    );
>    fatal_directive_status(
>      status,
> -    RTEMS_UNSATISFIED,
> -    "rtems_message_queue_create unsatisfied"
> +    RTEMS_INVALID_SIZE,
> +    "rtems_message_queue_create invalid size"
>    );
> -  puts( "TA1 - rtems_message_queue_create - Q 2 - RTEMS_UNSATISFIED #2" );
> +  puts( "TA1 - rtems_message_queue_create - Q 2 - RTEMS_INVALID_SIZE" );
>
>    status = rtems_message_queue_create(
>      Queue_name[ 1 ],
> --
> 2.26.2
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list