Re: [lng-odp] [PATCH] helper: tables: avoid invalid odp_shm_addr() calls

[Maxim Uvarov]

On 05/05/2017 03:49 PM, Matias Elo wrote:
odp_shm_lookup() return value can be used to detect name conflicts.
Signed-off-by: Matias Elo <matias....@nokia.com>
---
  helper/hashtable.c   | 3 +--
  helper/lineartable.c | 3 +--
  2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/helper/hashtable.c b/helper/hashtable.c
index 9d41144..f26b18b 100644
--- a/helper/hashtable.c
+++ b/helper/hashtable.c
@@ -76,8 +76,7 @@ odph_table_t odph_hash_table_create(const char *name, 
uint32_t capacity,
                ODPH_DBG("create para input error!\n");
                return NULL;
        }
-       tbl = (odph_hash_table_imp *)odp_shm_addr(odp_shm_lookup(name));
-       if (tbl != NULL) {
+       if (odp_shm_lookup(name) != ODP_SHM_INVALID) {
                ODPH_DBG("name already exist\n");
                return NULL;
        }
diff --git a/helper/lineartable.c b/helper/lineartable.c
index 32c4956..dd4a599 100644
--- a/helper/lineartable.c
+++ b/helper/lineartable.c
@@ -55,8 +55,7 @@ odph_table_t odph_linear_table_create(const char *name, 
uint32_t capacity,
                return NULL;
        }
        /* check name confict in shm*/
-       tbl = (odph_linear_table_imp *)odp_shm_addr(odp_shm_lookup(name));
-       if (tbl != NULL) {
+       if (odp_shm_lookup(name) != ODP_SHM_INVALID) {
                ODPH_DBG("name already exist\n");
                return NULL;
        }
Patch itself is ok,  it might be reasonable to rewrite the latest chunk 
in more readable way (if you already touched that chunk):

Instead of:

    shm = odp_shm_lookup(name);
    if (shm != ODP_SHM_INVALID)
        hash_tbl = (odph_hash_table_imp *)odp_shm_addr(shm);
    if (hash_tbl != NULL && strcmp(hash_tbl->name, name) == 0)
        return (odph_table_t)hash_tbl;
    return NULL;
}

Write:

    shm = odp_shm_lookup(name);
    if (shm == ODP_SHM_INVALID)
        return NULL;

    hash_tbl = (odph_hash_table_imp *)odp_shm_addr(shm);
     if (hash_tbl == NULL ||  strcmp(hash_tbl->name, name) != 0)
              return NULL;
    return (odph_table_t)hash_tbl;
}