lib: rte_*_create gives NULL/EEXIST on duped name
PATCH 1/1 lib: rte_*_create gives NULL/EEXIST on duped name
Also fixed a bug in many of them where if the rte_malloc of the TAILQ fails, then we return a pointer to some arbitrary existing struct.
--- lib/librte_acl/rte_acl.c | 53 +++++++++++++++++++++------------------ lib/librte_hash/rte_cuckoo_hash.c | 6 +++-- lib/librte_hash/rte_fbk_hash.c | 5 +++- lib/librte_lpm/rte_lpm.c | 5 +++- lib/librte_lpm/rte_lpm6.c | 5 +++- 5 files changed, 44 insertions(+), 30 deletions(-)
lib/librte_acl/rte_acl.c +28 -25
@@ -213,37 +213,40 @@ rte_acl_create(const struct rte_acl_param *param)break ; } + ctx =NULL ; +if (te !=NULL ) { + rte_errno =EEXIST ; + goto exit; + } +/* if ACL with such name doesn't exist, then create a new one. */ -if (te ==NULL ) { - ctx =NULL ; - te =rte_zmalloc ("ACL_TAILQ_ENTRY" ,sizeof (*te),0 ); + te =rte_zmalloc ("ACL_TAILQ_ENTRY" ,sizeof (*te),0 ); -if (te ==NULL ) { -RTE_LOG (ERR ,ACL ,"Cannot allocate tailq entry!\n" ); - goto exit; - } +if (te ==NULL ) { +RTE_LOG (ERR ,ACL ,"Cannot allocate tailq entry!\n" ); + goto exit; + } - ctx =rte_zmalloc_socket (name, sz,RTE_CACHE_LINE_SIZE , param->socket_id); + ctx =rte_zmalloc_socket (name, sz,RTE_CACHE_LINE_SIZE , param->socket_id); -if (ctx ==NULL ) { -RTE_LOG (ERR ,ACL , -"allocation of %zu bytes on socket %d for %s failed\n" , - sz, param->socket_id, name); -rte_free (te); - goto exit; - } -/* init new allocated context. */ - ctx->rules = ctx +1 ; - ctx->max_rules = param->max_rule_num; - ctx->rule_sz = param->rule_size; - ctx->socket_id = param->socket_id; - ctx->alg = rte_acl_default_classify; -snprintf (ctx->name,sizeof (ctx->name),"%s" , param->name); +if (ctx ==NULL ) { +RTE_LOG (ERR ,ACL , +"allocation of %zu bytes on socket %d for %s failed\n" , + sz, param->socket_id, name); +rte_free (te); + goto exit; + } +/* init new allocated context. */ + ctx->rules = ctx +1 ; + ctx->max_rules = param->max_rule_num; + ctx->rule_sz = param->rule_size; + ctx->socket_id = param->socket_id; + ctx->alg = rte_acl_default_classify; +snprintf (ctx->name,sizeof (ctx->name),"%s" , param->name); - te->data = (void *) ctx; + te->data = (void *) ctx; -TAILQ_INSERT_TAIL (acl_list, te, next); - } +TAILQ_INSERT_TAIL (acl_list, te, next); exit:rte_rwlock_write_unlock (RTE_EAL_TAILQ_RWLOCK );
lib/librte_hash/rte_cuckoo_hash.c +4 -2
@@ -206,8 +206,10 @@ rte_hash_create(const struct rte_hash_parameters *params)/* Guarantee there's no existing */ h =rte_hash_find_existing (params->name); -if (h !=NULL ) -return h; +if (h !=NULL ) { + rte_errno =EEXIST ; +return NULL ; + } te =rte_zmalloc ("HASH_TAILQ_ENTRY" ,sizeof (*te),0 );if (te ==NULL ) {
lib/librte_hash/rte_fbk_hash.c +4 -1
@@ -140,8 +140,11 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params)if (strncmp (params->name, ht->name,RTE_FBK_HASH_NAMESIZE ) ==0 )break ; } -if (te !=NULL ) + ht =NULL ; +if (te !=NULL ) { + rte_errno =EEXIST ; goto exit; + } te =rte_zmalloc ("FBK_HASH_TAILQ_ENTRY" ,sizeof (*te),0 );if (te ==NULL ) {
lib/librte_lpm/rte_lpm.c +4 -1
@@ -181,8 +181,11 @@ rte_lpm_create(const char *name, int socket_id, int max_rules,if (strncmp (name, lpm->name,RTE_LPM_NAMESIZE ) ==0 )break ; } -if (te !=NULL ) + lpm =NULL ; +if (te !=NULL ) { + rte_errno =EEXIST ; goto exit; + }/* allocate tailq entry */ te =rte_zmalloc ("LPM_TAILQ_ENTRY" ,sizeof (*te),0 );
lib/librte_lpm/rte_lpm6.c +4 -1
@@ -182,8 +182,11 @@ rte_lpm6_create(const char *name, int socket_id,if (strncmp (name, lpm->name,RTE_LPM6_NAMESIZE ) ==0 )break ; } -if (te !=NULL ) + lpm =NULL ; +if (te !=NULL ) { + rte_errno =EEXIST ; goto exit; + }/* allocate tailq entry */ te =rte_zmalloc ("LPM6_TAILQ_ENTRY" ,sizeof (*te),0 );
Signature · 1 line
2.6.2
Quoted 5 lines · Yoni Fogel
Also fixed a bug in many of them where if the
rte_mallocof the TAILQ fails, then we return a pointer to some arbitrary existing struct.Signed-off-by Yoni Fogel <[email protected]>
Thanks, looks like good fixes. However, I think for fixes like these they are better as one-fix-per-patch, because in this case we have the title referring to one fix, while the comment log describes a completely separate fix. Can you perhaps do a V2 as a two-patch set?
/Bruce
Quoted 8 lines
--- lib/librte_acl/rte_acl.c | 53 +++++++++++++++++++++------------------ lib/librte_hash/rte_cuckoo_hash.c | 6 +++-- lib/librte_hash/rte_fbk_hash.c | 5 +++- lib/librte_lpm/rte_lpm.c | 5 +++- lib/librte_lpm/rte_lpm6.c | 5 +++- 5 files changed, 44 insertions(+), 30 deletions(-)
Quoted 13 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Bruce Richardson
Sent: Wednesday, October 21, 2015 11:59 AM
To: Yoni Fogel
Cc: [email protected]
Subject: Re: [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped nameQuoted 3 lines · Yoni Fogel
Also fixed a bug in many of them where if the
rte_mallocof the TAILQ fails, then we return a pointer to some arbitrary existing struct.
Another nit - please don't forget to update comments in the .h files, about changed behaviour. Konstantin
Quoted 18 lines
Quoted 2 lines
Signed-off-by Yoni Fogel <[email protected]>Thanks, looks like good fixes. However, I think for fixes like these they are better as one-fix-per-patch, because in this case we have the title referring to one fix, while the comment log describes a completely separate fix. Can you perhaps do a V2 as a two-patch set?
/Bruce
Quoted 8 lines
--- lib/librte_acl/rte_acl.c | 53 +++++++++++++++++++++------------------ lib/librte_hash/rte_cuckoo_hash.c | 6 +++-- lib/librte_hash/rte_fbk_hash.c | 5 +++- lib/librte_lpm/rte_lpm.c | 5 +++- lib/librte_lpm/rte_lpm6.c | 5 +++- 5 files changed, 44 insertions(+), 30 deletions(-)