On 05/11/2017 03:06 PM, Elo, Matias (Nokia - FI/Espoo) wrote:
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;
}
True, however cleanup should be done in another patch. I'm not touching any 
other code in hashtable/lineartable. I found this bug while implementing native 
dpdk shm for odp-dpdk.

-Matias

Ok. Merged. But it's better to do cleanups when you touch lines. In other case we will never have cleaned up code.

Maxim.

Reply via email to