[PATCH] rtems: Fix rtems_partition_return_buffer()

[Gedare Bloom]

looks good, maybe one small clarification:

On Tue, Aug 10, 2021 at 4:44 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
> The rtems_partition_return_buffer() wrongly accepted which were exactly
> at the buffer area end.  Use the buffer area limit address for the range
> checking.
>
> Close #4490.
> ---
>  cpukit/include/rtems/monitor.h        |  2 +-
>  cpukit/include/rtems/rtems/partdata.h |  9 ++++-----
>  cpukit/libmisc/monitor/mon-part.c     |  5 +++--
>  cpukit/rtems/src/partcreate.c         |  8 ++++++--
>  cpukit/rtems/src/partreturnbuffer.c   | 17 ++++++++++-------
>  5 files changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/cpukit/include/rtems/monitor.h b/cpukit/include/rtems/monitor.h
> index d0a79c03be..9367e2b6e8 100644
> --- a/cpukit/include/rtems/monitor.h
> +++ b/cpukit/include/rtems/monitor.h
> @@ -192,7 +192,7 @@ typedef struct {
>      rtems_name          name;
>    /* end of common portion */
>    rtems_attribute     attribute;
> -  void *              start_addr;
> +  const void *        start_addr;
>    uint32_t            length;
>    uint32_t            buf_size;
>    uint32_t            used_blocks;
> diff --git a/cpukit/include/rtems/rtems/partdata.h b/cpukit/include/rtems/rtems/partdata.h
> index 4f4132ac6b..4c4eca3d17 100644
> --- a/cpukit/include/rtems/rtems/partdata.h
> +++ b/cpukit/include/rtems/rtems/partdata.h
> @@ -50,15 +50,14 @@ typedef struct {
>  #endif
>
>    /**
> -   * @brief This member contains the physical starting address of the buffer
> -   *   area.
> +   * @brief This member contains the base address of the buffer area.
Probably it is clear, but could add here "This is the address of the
first valid byte in the buffer." to mirror the suggestion below that
clarifies the meaning of limit.

>     */
> -  void *starting_address;
> +  const void *base_address;
>
>    /**
> -   * @brief This member contains the size of the buffer area in bytes.
> +   * @brief This member contains the limit address of the buffer area.
Maybe add: "This is the address of the last valid byte in the buffer."

>     */
> -  uintptr_t length;
> +  const void *limit_address;
>
>    /**
>     * @brief This member contains the size of each buffer in bytes.
> diff --git a/cpukit/libmisc/monitor/mon-part.c b/cpukit/libmisc/monitor/mon-part.c
> index 18034cd58f..654700ebfc 100644
> --- a/cpukit/libmisc/monitor/mon-part.c
> +++ b/cpukit/libmisc/monitor/mon-part.c
> @@ -22,8 +22,9 @@ rtems_monitor_part_canonical(
>      const Partition_Control *rtems_part = (const Partition_Control *) part_void;
>
>      canonical_part->attribute = rtems_part->attribute_set;
> -    canonical_part->start_addr = rtems_part->starting_address;
> -    canonical_part->length = rtems_part->length;
> +    canonical_part->start_addr = rtems_part->base_address;
> +    canonical_part->length = (uint32_t) ( (uintptr_t)
> +        rtems_part->limit_address + 1 - (uintptr_t) rtems_part->base_address );
>      canonical_part->buf_size = rtems_part->buffer_size;
>      canonical_part->used_blocks = rtems_part->number_of_used_blocks;
>  }
> diff --git a/cpukit/rtems/src/partcreate.c b/cpukit/rtems/src/partcreate.c
> index 012a416a1a..61249749f3 100644
> --- a/cpukit/rtems/src/partcreate.c
> +++ b/cpukit/rtems/src/partcreate.c
> @@ -23,6 +23,7 @@
>  #include <rtems/rtems/partimpl.h>
>  #include <rtems/rtems/attrimpl.h>
>  #include <rtems/rtems/support.h>
> +#include <rtems/score/address.h>
>  #include <rtems/score/chainimpl.h>
>  #include <rtems/score/sysstate.h>
>  #include <rtems/sysinit.h>
> @@ -40,8 +41,11 @@ static void _Partition_Initialize(
>    rtems_attribute    attribute_set
>  )
>  {
> -  the_partition->starting_address      = starting_address;
> -  the_partition->length                = length;
> +  const void *limit_address;
> +
> +  limit_address = _Addresses_Add_offset( starting_address, length - 1 );
> +  the_partition->base_address          = starting_address;
> +  the_partition->limit_address         = limit_address;
>    the_partition->buffer_size           = buffer_size;
>    the_partition->attribute_set         = attribute_set;
>    the_partition->number_of_used_blocks = 0;
> diff --git a/cpukit/rtems/src/partreturnbuffer.c b/cpukit/rtems/src/partreturnbuffer.c
> index f5ab7d85f9..68302f1163 100644
> --- a/cpukit/rtems/src/partreturnbuffer.c
> +++ b/cpukit/rtems/src/partreturnbuffer.c
> @@ -33,7 +33,7 @@ static bool _Partition_Is_address_on_buffer_boundary(
>
>    offset = _Addresses_Subtract(
>      the_buffer,
> -    the_partition->starting_address
> +    the_partition->base_address
>    );
>
>    return ( offset % the_partition->buffer_size ) == 0;
> @@ -44,14 +44,17 @@ static bool _Partition_Is_address_a_buffer_begin(
>     const void              *the_buffer
>  )
>  {
> -  void *starting;
> -  void *ending;
> +  const void *base;
> +  const void *limit;
>
> -  starting = the_partition->starting_address;
> -  ending   = _Addresses_Add_offset( starting, the_partition->length );
> +  base = the_partition->base_address;
> +  limit = the_partition->limit_address;
>
> -  return _Addresses_Is_in_range( the_buffer, starting, ending )
> -    && _Partition_Is_address_on_buffer_boundary( the_partition, the_buffer );
> +  if ( !_Addresses_Is_in_range( the_buffer, base, limit ) ) {
> +    return false;
> +  }
> +
> +  return _Partition_Is_address_on_buffer_boundary( the_partition, the_buffer );
>  }
>
>  static void _Partition_Free_buffer(
> --
> 2.26.2
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel