On 19 November 2014 10:36, Yan Songming <[email protected]> wrote:

> New API implementing odp_shm_free to match the odp_shm_reserve.
>
> Signed-off-by: Yan Songming <[email protected]>
> ---
> v3 change the return value of odp_shm_free.
> v2 fix the problem which Maxim found.
> ---
>  .../linux-generic/include/api/odp_shared_memory.h  |  2 +-
>  platform/linux-generic/odp_shared_memory.c         | 43
> +++++++++++++++++++---
>  2 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/platform/linux-generic/include/api/odp_shared_memory.h
> b/platform/linux-generic/include/api/odp_shared_memory.h
> index ff6f9a9..d42e272 100644
> --- a/platform/linux-generic/include/api/odp_shared_memory.h
> +++ b/platform/linux-generic/include/api/odp_shared_memory.h
> @@ -81,7 +81,7 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>   * @param[in] shm Block handle
>   *
>

>From the implementation below we can be more precise about the behaviour of
this API


>   * @retval 0 for success
>

@retval 0 if the handle is already free
@retval 0 if the handle free succeeds


> - * @retval 1 on failure
> + * @retval -1 on failure
>

@retval -1 on failure to free the handle


>   */
>  int odp_shm_free(odp_shm_t shm);
>
> diff --git a/platform/linux-generic/odp_shared_memory.c
> b/platform/linux-generic/odp_shared_memory.c
> index 24a5d60..bc0ab55 100644
> --- a/platform/linux-generic/odp_shared_memory.c
> +++ b/platform/linux-generic/odp_shared_memory.c
> @@ -114,6 +114,43 @@ static int find_block(const char *name, uint32_t
> *index)
>         return 0;
>  }
>
> +int odp_shm_free(odp_shm_t shm)
> +{
> +       uint32_t i;
> +       int ret;
> +       odp_shm_block_t *shm_block;
> +       uint64_t alloc_size;
> +
> +       i = from_handle(shm);
> +       if (odp_shm_tbl->block[i].addr == NULL) {
>

Would it be better to follow the slightly better habit of reversing the
constant with the variable

NULL == odp_shm_tbl->block[i].addr


> +               /* free block */
>

Is this comment adding value ?


> +               ODP_DBG("odp_shm_free: Free block\n");
> +               return 0;
> +       }
> +
> +       odp_spinlock_lock(&odp_shm_tbl->lock);
> +       shm_block = &odp_shm_tbl->block[i];
> +
> +       alloc_size = shm_block->size + shm_block->align;
> +       ret = munmap(shm_block->addr, alloc_size);
> +       if (0 != ret) {
> +               ODP_DBG("odp_shm_free: munmap failed\n");
> +               odp_spinlock_unlock(&odp_shm_tbl->lock);
> +               return -1;
> +       }
> +
> +       if (shm_block->flags & ODP_SHM_PROC) {
> +               ret = shm_unlink(shm_block->name);
> +               if (0 != ret) {
> +                       ODP_DBG("odp_shm_free: shm_unlink failed\n");
> +                       odp_spinlock_unlock(&odp_shm_tbl->lock);
> +                       return -1;
> +               }
> +       }
> +       memset(&odp_shm_tbl->block[i], 0, sizeof(odp_shm_block_t));
> +       odp_spinlock_unlock(&odp_shm_tbl->lock);
> +       return 0;
> +}
>
>  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
>                           uint32_t flags)
> @@ -221,12 +258,6 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>         return block->hdl;
>  }
>
> -int odp_shm_free(odp_shm_t shm ODP_UNUSED)
> -{
> -       ODP_UNIMPLEMENTED();
> -       return 0;
> -}
> -
>  odp_shm_t odp_shm_lookup(const char *name)
>  {
>         uint32_t i;
> --
> 1.8.3.1
>
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
*Mike Holmes*
Linaro  Sr Technical Manager
LNG - ODP
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to