Re: [dpdk-dev] [PATCH v2 1/6] hash: make duplicated code into functions

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

Hi Yipeng,

> -----Original Message-----
> From: Wang, Yipeng1
> Sent: Friday, June 29, 2018 1:25 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; Wang, Yipeng1 <yipeng1.wang@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; honnappa.nagarahalli@arm.com;
> vguvva@caviumnetworks.com; brijesh.s.singh@gmail.com
> Subject: [PATCH v2 1/6] hash: make duplicated code into functions
> 
> This commit refactors the hash table lookup/add/del code to remove some code
> duplication. Processing on primary bucket can also apply to secondary bucket
> with same code.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  lib/librte_hash/rte_cuckoo_hash.c | 186 ++++++++++++++++++--------------------

...

> +/* Search one bucket to find the match key */
>  static inline int32_t
> -__rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key,
> -					hash_sig_t sig, void **data)
> +search_one_bucket(const struct rte_hash *h, const void *key, hash_sig_t sig,
> +			void **data, struct rte_hash_bucket *bkt)

Use "const" in "struct rte_hash_bucket".

...

> +search_and_remove(const struct rte_hash *h, const void *key,
> +			struct rte_hash_bucket *bkt, hash_sig_t sig,
> +			int32_t *ret_val)
>  {
> -	uint32_t bucket_idx;
> -	hash_sig_t alt_hash;
> -	unsigned i;
> -	struct rte_hash_bucket *bkt;
>  	struct rte_hash_key *k, *keys = h->key_store;
> -	int32_t ret;
> -
> -	bucket_idx = sig & h->bucket_bitmask;
> -	bkt = &h->buckets[bucket_idx];
> +	unsigned int i;
> 
>  	/* Check if key is in primary location */
>  	for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) { @@ -833,37 +825,39
> @@ __rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key,
>  				 * Return index where key is stored,
>  				 * subtracting the first dummy index
>  				 */
> -				ret = bkt->key_idx[i] - 1;
> +				*ret_val = bkt->key_idx[i] - 1;
>  				bkt->key_idx[i] = EMPTY_SLOT;
> -				return ret;
> +				return 0;

You can store ret_val and return it, instead of returning 0,
so the function is similar to the other search functions.


>  			}
>  		}
>  	}
> +	return -1;