[dpdk-dev] [PATCH 1/6] test: remove unneeded casts

[dpdk-dev] [PATCH 1/6] test: remove unneeded casts

[Stephen Hemminger]

The malloc family returns void * and therefore cast is unnecessary.
Use calloc rather than zmalloc with multiply for array.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_hash_perf.c | 8 ++++----
 app/test/test_mempool.c   | 2 +-
 app/test/test_ring.c      | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/test/test_hash_perf.c b/app/test/test_hash_perf.c
index be34957..6f719fc 100644
--- a/app/test/test_hash_perf.c
+++ b/app/test/test_hash_perf.c
@@ -459,13 +459,13 @@ run_single_tbl_perf_test(const struct rte_hash *h, hash_operation func,
 
 	/* Initialise */
 	num_buckets = params->entries / params->bucket_entries;
-	key = (uint8_t *) rte_zmalloc("hash key",
-			params->key_len * sizeof(uint8_t), 16);
+	key = rte_zmalloc("hash key",
+			  params->key_len * sizeof(uint8_t), 16);
 	if (key == NULL)
 		return -1;
 
-	bucket_occupancies = (uint32_t *) rte_zmalloc("bucket occupancies",
-			num_buckets * sizeof(uint32_t), 16);
+	bucket_occupancies = rte_calloc("bucket occupancies",
+					num_buckets, sizeof(uint32_t), 16);
 	if (bucket_occupancies == NULL) {
 		rte_free(key);
 		return -1;
diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 303d2b3..de85c9c 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -360,7 +360,7 @@ test_mempool_basic_ex(struct rte_mempool * mp)
 	if (mp == NULL)
 		return ret;
 
-	obj = (void **)rte_zmalloc("test_mempool_basic_ex", (MEMPOOL_SIZE * sizeof(void *)), 0);
+	obj = rte_calloc("test_mempool_basic_ex", MEMPOOL_SIZE , sizeof(void *), 0);
 	if (obj == NULL) {
 		printf("test_mempool_basic_ex fail to rte_malloc\n");
 		return ret;
diff --git a/app/test/test_ring.c b/app/test/test_ring.c
index 2cd8e77..ce25329 100644
--- a/app/test/test_ring.c
+++ b/app/test/test_ring.c
@@ -1259,7 +1259,7 @@ test_ring_basic_ex(void)
 	struct rte_ring * rp;
 	void **obj = NULL;
 
-	obj = (void **)rte_zmalloc("test_ring_basic_ex_malloc", (RING_SIZE * sizeof(void *)), 0);
+	obj = rte_calloc("test_ring_basic_ex_malloc", RING_SIZE, sizeof(void *), 0);
 	if (obj == NULL) {
 		printf("test_ring_basic_ex fail to rte_malloc\n");
 		goto fail_test;
-- 
2.1.4

[dpdk-dev] [PATCH 2/6] vhost_xen: remove unnecessary cast

[Stephen Hemminger]

Don't need to cast malloc family of functions since they return
void *.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/vhost_xen/vhost_monitor.c  | 2 +-
 examples/vhost_xen/xenstore_parse.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/examples/vhost_xen/vhost_monitor.c b/examples/vhost_xen/vhost_monitor.c
index f683989..9d99962 100644
--- a/examples/vhost_xen/vhost_monitor.c
+++ b/examples/vhost_xen/vhost_monitor.c
@@ -138,7 +138,7 @@ add_xen_guest(int32_t dom_id)
 	if ((guest = get_xen_guest(dom_id)) != NULL)
 		return guest;
 
-	guest = (struct xen_guest * )calloc(1, sizeof(struct xen_guest));
+	guest = calloc(1, sizeof(struct xen_guest));
 	if (guest) {
 		RTE_LOG(ERR, XENHOST, "  %s: return newly created guest with %d rings\n", __func__, guest->vring_num);
 		TAILQ_INSERT_TAIL(&guest_root, guest, next);
diff --git a/examples/vhost_xen/xenstore_parse.c b/examples/vhost_xen/xenstore_parse.c
index 9441639..df191ac 100644
--- a/examples/vhost_xen/xenstore_parse.c
+++ b/examples/vhost_xen/xenstore_parse.c
@@ -248,8 +248,8 @@ parse_gntnode(int dom_id, char *path)
 		goto err;
 	}
 
-	gntnode = (struct xen_gntnode *)calloc(1, sizeof(struct xen_gntnode));
-	gnt = (struct xen_gnt *)calloc(gref_num, sizeof(struct xen_gnt));
+	gntnode = calloc(1, sizeof(struct xen_gntnode));
+	gnt = calloc(gref_num, sizeof(struct xen_gnt));
 	if (gnt == NULL || gntnode == NULL)
 		goto err;
 
-- 
2.1.4

[dpdk-dev] [PATCH 3/6] bsd: remove useless assignments

[Stephen Hemminger]

If variable is set in the next line, it doesn't need to be
initialized.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/bsdapp/eal/eal.c     | 3 ++-
 lib/librte_eal/bsdapp/eal/eal_pci.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 69f3c03..71ae33c 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -417,7 +417,8 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_iopl_init(void)
 {
-	int fd = -1;
+	int fd;
+
 	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 74ecce7..d191323 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -382,7 +382,7 @@ skipdev:
 static int
 pci_scan(void)
 {
-	int fd = -1;
+	int fd;
 	unsigned dev_count = 0;
 	struct pci_conf matches[16];
 	struct pci_conf_io conf_io = {
-- 
2.1.4

[dpdk-dev] [PATCH 4/6] enic: eliminate useless cast

[Stephen Hemminger]

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_pmd_enic/enic_clsf.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_enic/enic_clsf.c b/lib/librte_pmd_enic/enic_clsf.c
index 577a382..b61d625 100644
--- a/lib/librte_pmd_enic/enic_clsf.c
+++ b/lib/librte_pmd_enic/enic_clsf.c
@@ -121,9 +121,8 @@ int enic_fdir_add_fltr(struct enic *enic, struct rte_fdir_filter *params,
 			enic->fdir.stats.f_add++;
 			return -ENOSPC;
 		}
-		key = (struct enic_fdir_node *)rte_zmalloc(
-			"enic_fdir_node",
-			sizeof(struct enic_fdir_node), 0);
+		key = rte_zmalloc("enic_fdir_node",
+				  sizeof(struct enic_fdir_node), 0);
 		if (!key) {
 			enic->fdir.stats.f_add++;
 			return -ENOMEM;
-- 
2.1.4

[dpdk-dev] [PATCH 5/6] eal: remove useless memset

[Stephen Hemminger]

The path variable is set via snprintf, and does not need to
memset before that.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index 590cb56..8d29e06 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -84,8 +84,6 @@ get_num_hugepages(const char *subdir)
 	else
 		nr_hp_file = "free_hugepages";
 
-	memset(path, 0, sizeof(path));
-
 	snprintf(path, sizeof(path), "%s/%s/%s",
 			sys_dir_path, subdir, nr_hp_file);
 
-- 
2.1.4

[dpdk-dev] [PATCH 6/6] examples: remove unneeded casts

[Stephen Hemminger]

*alloc() routines return void * and therefore cast is not needed.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 examples/kni/main.c       | 4 ++--
 examples/l3fwd-acl/main.c | 4 ++--
 examples/vhost/main.c     | 7 ++++---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/examples/kni/main.c b/examples/kni/main.c
index 45b96bc..2bff1e1 100644
--- a/examples/kni/main.c
+++ b/examples/kni/main.c
@@ -462,8 +462,8 @@ parse_config(const char *arg)
 			goto fail;
 		}
 		kni_port_params_array[port_id] =
-			(struct kni_port_params*)rte_zmalloc("KNI_port_params",
-			sizeof(struct kni_port_params), RTE_CACHE_LINE_SIZE);
+			rte_zmalloc("KNI_port_params",
+				    sizeof(struct kni_port_params), RTE_CACHE_LINE_SIZE);
 		kni_port_params_array[port_id]->port_id = port_id;
 		kni_port_params_array[port_id]->lcore_rx =
 					(uint8_t)int_fld[i++];
diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
index f1f7601..20e071a 100644
--- a/examples/l3fwd-acl/main.c
+++ b/examples/l3fwd-acl/main.c
@@ -1047,13 +1047,13 @@ add_rules(const char *rule_path,
 
 	fseek(fh, 0, SEEK_SET);
 
-	acl_rules = (uint8_t *)calloc(acl_num, rule_size);
+	acl_rules = calloc(acl_num, rule_size);
 
 	if (NULL == acl_rules)
 		rte_exit(EXIT_FAILURE, "%s: failed to malloc memory\n",
 			__func__);
 
-	route_rules = (uint8_t *)calloc(route_num, rule_size);
+	route_rules = calloc(route_num, rule_size);
 
 	if (NULL == route_rules)
 		rte_exit(EXIT_FAILURE, "%s: failed to malloc memory\n",
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 3a35359..a96b19f 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -2592,9 +2592,10 @@ new_device (struct virtio_net *dev)
 
 		}
 
-		vdev->regions_hpa = (struct virtio_memory_regions_hpa *) rte_zmalloc("vhost hpa region",
-			sizeof(struct virtio_memory_regions_hpa) * vdev->nregions_hpa,
-			RTE_CACHE_LINE_SIZE);
+		vdev->regions_hpa = rte_calloc("vhost hpa region",
+					       sizeof(struct virtio_memory_regions_hpa),
+					       vdev->nregions_hpa,
+					       RTE_CACHE_LINE_SIZE);
 		if (vdev->regions_hpa == NULL) {
 			RTE_LOG(ERR, VHOST_CONFIG, "Cannot allocate memory for hpa region\n");
 			rte_free(vdev);
-- 
2.1.4

Re: [dpdk-dev] [PATCH 3/6] bsd: remove useless assignments

[Bruce Richardson]

On Sat, Feb 14, 2015 at 09:59:07AM -0500, Stephen Hemminger wrote:
> If variable is set in the next line, it doesn't need to be
> initialized.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_eal/bsdapp/eal/eal.c     | 3 ++-
>  lib/librte_eal/bsdapp/eal/eal_pci.c | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 69f3c03..71ae33c 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -417,7 +417,8 @@ int rte_eal_has_hugepages(void)
>  int
>  rte_eal_iopl_init(void)
>  {
> -	int fd = -1;
> +	int fd;
> +
>  	fd = open("/dev/io", O_RDWR);

Why not just merge the two lines and make it "int fd = open(...);". 

/Bruce

Re: [dpdk-dev] [PATCH 1/6] test: remove unneeded casts

[Bruce Richardson]

On Sat, Feb 14, 2015 at 09:59:05AM -0500, Stephen Hemminger wrote:
> The malloc family returns void * and therefore cast is unnecessary.
> Use calloc rather than zmalloc with multiply for array.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Looks like a good basic cleanup

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  app/test/test_hash_perf.c | 8 ++++----
>  app/test/test_mempool.c   | 2 +-
>  app/test/test_ring.c      | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test/test_hash_perf.c b/app/test/test_hash_perf.c
> index be34957..6f719fc 100644
> --- a/app/test/test_hash_perf.c
> +++ b/app/test/test_hash_perf.c
> @@ -459,13 +459,13 @@ run_single_tbl_perf_test(const struct rte_hash *h, hash_operation func,
>  
>  	/* Initialise */
>  	num_buckets = params->entries / params->bucket_entries;
> -	key = (uint8_t *) rte_zmalloc("hash key",
> -			params->key_len * sizeof(uint8_t), 16);
> +	key = rte_zmalloc("hash key",
> +			  params->key_len * sizeof(uint8_t), 16);
>  	if (key == NULL)
>  		return -1;
>  
> -	bucket_occupancies = (uint32_t *) rte_zmalloc("bucket occupancies",
> -			num_buckets * sizeof(uint32_t), 16);
> +	bucket_occupancies = rte_calloc("bucket occupancies",
> +					num_buckets, sizeof(uint32_t), 16);
>  	if (bucket_occupancies == NULL) {
>  		rte_free(key);
>  		return -1;
> diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
> index 303d2b3..de85c9c 100644
> --- a/app/test/test_mempool.c
> +++ b/app/test/test_mempool.c
> @@ -360,7 +360,7 @@ test_mempool_basic_ex(struct rte_mempool * mp)
>  	if (mp == NULL)
>  		return ret;
>  
> -	obj = (void **)rte_zmalloc("test_mempool_basic_ex", (MEMPOOL_SIZE * sizeof(void *)), 0);
> +	obj = rte_calloc("test_mempool_basic_ex", MEMPOOL_SIZE , sizeof(void *), 0);
>  	if (obj == NULL) {
>  		printf("test_mempool_basic_ex fail to rte_malloc\n");
>  		return ret;
> diff --git a/app/test/test_ring.c b/app/test/test_ring.c
> index 2cd8e77..ce25329 100644
> --- a/app/test/test_ring.c
> +++ b/app/test/test_ring.c
> @@ -1259,7 +1259,7 @@ test_ring_basic_ex(void)
>  	struct rte_ring * rp;
>  	void **obj = NULL;
>  
> -	obj = (void **)rte_zmalloc("test_ring_basic_ex_malloc", (RING_SIZE * sizeof(void *)), 0);
> +	obj = rte_calloc("test_ring_basic_ex_malloc", RING_SIZE, sizeof(void *), 0);
>  	if (obj == NULL) {
>  		printf("test_ring_basic_ex fail to rte_malloc\n");
>  		goto fail_test;
> -- 
> 2.1.4
> 

Re: [dpdk-dev] [PATCH 2/6] vhost_xen: remove unnecessary cast

[Bruce Richardson]

On Sat, Feb 14, 2015 at 09:59:06AM -0500, Stephen Hemminger wrote:
> Don't need to cast malloc family of functions since they return
> void *.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  examples/vhost_xen/vhost_monitor.c  | 2 +-
>  examples/vhost_xen/xenstore_parse.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/vhost_xen/vhost_monitor.c b/examples/vhost_xen/vhost_monitor.c
> index f683989..9d99962 100644
> --- a/examples/vhost_xen/vhost_monitor.c
> +++ b/examples/vhost_xen/vhost_monitor.c
> @@ -138,7 +138,7 @@ add_xen_guest(int32_t dom_id)
>  	if ((guest = get_xen_guest(dom_id)) != NULL)
>  		return guest;
>  
> -	guest = (struct xen_guest * )calloc(1, sizeof(struct xen_guest));
> +	guest = calloc(1, sizeof(struct xen_guest));
>  	if (guest) {
>  		RTE_LOG(ERR, XENHOST, "  %s: return newly created guest with %d rings\n", __func__, guest->vring_num);
>  		TAILQ_INSERT_TAIL(&guest_root, guest, next);
> diff --git a/examples/vhost_xen/xenstore_parse.c b/examples/vhost_xen/xenstore_parse.c
> index 9441639..df191ac 100644
> --- a/examples/vhost_xen/xenstore_parse.c
> +++ b/examples/vhost_xen/xenstore_parse.c
> @@ -248,8 +248,8 @@ parse_gntnode(int dom_id, char *path)
>  		goto err;
>  	}
>  
> -	gntnode = (struct xen_gntnode *)calloc(1, sizeof(struct xen_gntnode));
> -	gnt = (struct xen_gnt *)calloc(gref_num, sizeof(struct xen_gnt));
> +	gntnode = calloc(1, sizeof(struct xen_gntnode));
> +	gnt = calloc(gref_num, sizeof(struct xen_gnt));
>  	if (gnt == NULL || gntnode == NULL)
>  		goto err;
>  
> -- 
> 2.1.4
> 

Re: [dpdk-dev] [PATCH 4/6] enic: eliminate useless cast

[Bruce Richardson]

On Sat, Feb 14, 2015 at 09:59:08AM -0500, Stephen Hemminger wrote:
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_pmd_enic/enic_clsf.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_pmd_enic/enic_clsf.c b/lib/librte_pmd_enic/enic_clsf.c
> index 577a382..b61d625 100644
> --- a/lib/librte_pmd_enic/enic_clsf.c
> +++ b/lib/librte_pmd_enic/enic_clsf.c
> @@ -121,9 +121,8 @@ int enic_fdir_add_fltr(struct enic *enic, struct rte_fdir_filter *params,
>  			enic->fdir.stats.f_add++;
>  			return -ENOSPC;
>  		}
> -		key = (struct enic_fdir_node *)rte_zmalloc(
> -			"enic_fdir_node",
> -			sizeof(struct enic_fdir_node), 0);
> +		key = rte_zmalloc("enic_fdir_node",
> +				  sizeof(struct enic_fdir_node), 0);
>  		if (!key) {
>  			enic->fdir.stats.f_add++;
>  			return -ENOMEM;
> -- 
> 2.1.4
> 

Re: [dpdk-dev] [PATCH 5/6] eal: remove useless memset

[Bruce Richardson]

On Sat, Feb 14, 2015 at 09:59:09AM -0500, Stephen Hemminger wrote:
> The path variable is set via snprintf, and does not need to
> memset before that.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> index 590cb56..8d29e06 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
> @@ -84,8 +84,6 @@ get_num_hugepages(const char *subdir)
>  	else
>  		nr_hp_file = "free_hugepages";
>  
> -	memset(path, 0, sizeof(path));
> -
>  	snprintf(path, sizeof(path), "%s/%s/%s",
>  			sys_dir_path, subdir, nr_hp_file);
>  
> -- 
> 2.1.4
> 

Re: [dpdk-dev] [PATCH 6/6] examples: remove unneeded casts

[Bruce Richardson]

On Sat, Feb 14, 2015 at 09:59:10AM -0500, Stephen Hemminger wrote:
> *alloc() routines return void * and therefore cast is not needed.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  examples/kni/main.c       | 4 ++--
>  examples/l3fwd-acl/main.c | 4 ++--
>  examples/vhost/main.c     | 7 ++++---
>  3 files changed, 8 insertions(+), 7 deletions(-)
> 
... <snip> ...
> diff --git a/examples/vhost/main.c b/examples/vhost/main.c
> index 3a35359..a96b19f 100644
> --- a/examples/vhost/main.c
> +++ b/examples/vhost/main.c
> @@ -2592,9 +2592,10 @@ new_device (struct virtio_net *dev)
>  
>  		}
>  
> -		vdev->regions_hpa = (struct virtio_memory_regions_hpa *) rte_zmalloc("vhost hpa region",
> -			sizeof(struct virtio_memory_regions_hpa) * vdev->nregions_hpa,
> -			RTE_CACHE_LINE_SIZE);
> +		vdev->regions_hpa = rte_calloc("vhost hpa region",
> +					       sizeof(struct virtio_memory_regions_hpa),
> +					       vdev->nregions_hpa,
> +					       RTE_CACHE_LINE_SIZE);

I know functionally it probably doesn't make a difference, but I think your
"num" and "size" parameters are reversed here.

/Bruce

Re: [dpdk-dev] [PATCH 1/6] test: remove unneeded casts

[Thomas Monjalon]

2015-02-19 14:53, Bruce Richardson:
> On Sat, Feb 14, 2015 at 09:59:05AM -0500, Stephen Hemminger wrote:
> > The malloc family returns void * and therefore cast is unnecessary.
> > Use calloc rather than zmalloc with multiply for array.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Looks like a good basic cleanup
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied the series despite the lack of answer to simple questions.