diff mbox

[dpdk-dev,1/2] Fix checkpatch errors in librte_acl

Message ID 1419521508-31883-2-git-send-email-rkerur@gmail.com (mailing list archive)
State Rejected, archived
Headers show

Commit Message

Ravi Kerur Dec. 25, 2014, 3:31 p.m. UTC
Fix checkpatch warnings and errors in lib/librte_acl. checkpatch
is run as follows

scripts/checkpatch.pl --no-tree --file <file_name>

Following warnings are treated as false-positive

1. WARNING: quoted string split across lines
2. WARNING: do not add new typedefs
3. WARNING: __aligned(size) is preferred over __attribute__((aligned(size)))

Signed-off-by: Ravi Kerur <rkerur@gmail.com>
---
 lib/librte_acl/acl_bld.c             | 192 +++++++++++++++++++----------------
 lib/librte_acl/rte_acl.c             |   3 +-
 lib/librte_acl/rte_acl_osdep_alone.h |   3 +-
 lib/librte_acl/tb_mem.c              |   5 +-
 4 files changed, 109 insertions(+), 94 deletions(-)

Comments

Bruce Richardson Jan. 5, 2015, 12:08 p.m. UTC | #1
On Thu, Dec 25, 2014 at 10:31:47AM -0500, Ravi Kerur wrote:
> Fix checkpatch warnings and errors in lib/librte_acl. checkpatch
> is run as follows
> 
> scripts/checkpatch.pl --no-tree --file <file_name>
> 
> Following warnings are treated as false-positive
> 
> 1. WARNING: quoted string split across lines
> 2. WARNING: do not add new typedefs
> 3. WARNING: __aligned(size) is preferred over __attribute__((aligned(size)))
> 
> Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> ---
>  lib/librte_acl/acl_bld.c             | 192 +++++++++++++++++++----------------
>  lib/librte_acl/rte_acl.c             |   3 +-
>  lib/librte_acl/rte_acl_osdep_alone.h |   3 +-
>  lib/librte_acl/tb_mem.c              |   5 +-
>  4 files changed, 109 insertions(+), 94 deletions(-)
> 
> diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
> index d6e0c45..1f60411 100644
> --- a/lib/librte_acl/acl_bld.c
> +++ b/lib/librte_acl/acl_bld.c
> @@ -773,11 +773,13 @@ acl_merge(struct acl_build_context *context,
>  		for (n = 0; n < ptrs_a; n++) {
>  			for (m = 0; m < ptrs_b; m++) {
>  
> +				uint32_t acl_intsct_type, num_cats;
> +
>  				if (node_a->ptrs[n].ptr == NULL ||
> -						node_b->ptrs[m].ptr == NULL ||
> -						node_a->ptrs[n].ptr ==
> -						node_b->ptrs[m].ptr)
> -						continue;
> +					node_b->ptrs[m].ptr == NULL ||
> +					node_a->ptrs[n].ptr ==
> +					node_b->ptrs[m].ptr)
> +					continue;
>  

This isn't a good fix, as the body of the if statement still lines up
with the continuation of the if statement itself. Instead, I think this should
just be fixed by moving the "continue" left by one tabstop. That would make
it correctly indended by one tab, vs previous level, while also ensuring that
it doesn't line up with the previous line continuations.

>  				intersect_type = acl_intersect_type(
>  					&node_a->ptrs[n].values,
> @@ -785,35 +787,38 @@ acl_merge(struct acl_build_context *context,
>  					&intersect_ptr);
>  
>  				/* If this node is not a 'match' node */
> -				if ((intersect_type & ACL_INTERSECT) &&
> -					(context->cfg.num_categories != 1 ||
> -					!(node_a->ptrs[n].ptr->match_flag))) {
> -
> -					/*
> -					 * next merge is a 'move' pointer,
> -					 * if this one is and B is a
> -					 * subset of the intersection.
> -					 */
> -					next_move = move &&
> -						(intersect_type &
> -						ACL_INTERSECT_B) == 0;
> -
> -					if (a_subset && b_full) {
> -						rc = acl_merge(context,
> -							node_a->ptrs[n].ptr,
> -							node_b->ptrs[m].ptr,
> -							next_move,
> -							1, level + 1);
> -						if (rc != 0)
> -							return rc;
> -					} else {
> -						rc = acl_merge_intersect(
> -							context, node_a, n,
> -							node_b, m, next_move,
> -							level, &intersect_ptr);
> -						if (rc != 0)
> -							return rc;
> -					}
> +				acl_intsct_type =
> +					intersect_type & ACL_INTERSECT;
> +				num_cats = (context->cfg.num_categories != 1 ||
> +					!(node_a->ptrs[n].ptr->match_flag));
> +
> +				if (!(acl_intsct_type && num_cats))
> +					continue;
> +
> +				/*
> +				 * next merge is a 'move' pointer,
> +				 * if this one is and B is a
> +				 * subset of the intersection.
> +				 */
> +				next_move = move &&
> +					(intersect_type &
> +					ACL_INTERSECT_B) == 0;
> +
> +				if (a_subset && b_full) {
> +					rc = acl_merge(context,
> +						node_a->ptrs[n].ptr,
> +						node_b->ptrs[m].ptr,
> +						next_move,
> +						1, level + 1);
> +					if (rc != 0)
> +						return rc;
> +				} else {
> +					rc = acl_merge_intersect(
> +						context, node_a, n,
> +						node_b, m, next_move,
> +						level, &intersect_ptr);
> +					if (rc != 0)
> +						return rc;
>  				}
>  			}
>  		}
> @@ -1099,52 +1104,52 @@ acl_merge_trie(struct acl_build_context *context,
>  					&node_b->ptrs[m].values,
>  					&child_intersect);
>  
> -				if ((child_intersect_type & ACL_INTERSECT) !=
> -						0) {
> -					if (acl_merge_trie(context,
> -							node_c->ptrs[n].ptr,
> -							node_b->ptrs[m].ptr,
> -							level + 1, subtree_id,
> -							&child_node_c))
> -						return 1;
> -
> -					if (child_node_c != NULL &&
> -							child_node_c !=
> -							node_c->ptrs[n].ptr) {
> -
> -						node_b_refs++;
> -
> -						/*
> -						 * Added link from C to
> -						 * child_C for all transitions
> -						 * in the intersection.
> -						 */
> -						acl_add_ptr(context, node_c,
> -							child_node_c,
> -							&child_intersect);
> -
> -						/*
> -						 * inc refs if pointer is not
> -						 * to node b.
> -						 */
> -						node_a_refs += (child_node_c !=
> -							node_b->ptrs[m].ptr);
> -
> -						/*
> -						 * Remove intersection from C
> -						 * pointer.
> -						 */
> -						if (!acl_exclude(
> -							&node_c->ptrs[n].values,
> -							&node_c->ptrs[n].values,
> -							&child_intersect)) {
> -							acl_deref_ptr(context,
> -								node_c, n);
> -							node_c->ptrs[n].ptr =
> -								NULL;
> -							node_a_refs--;
> -						}
> -					}
> +				if ((child_intersect_type & ACL_INTERSECT) ==
> +						0)
> +					continue;
> +
> +				if (acl_merge_trie(context,
> +						node_c->ptrs[n].ptr,
> +						node_b->ptrs[m].ptr,
> +						level + 1, subtree_id,
> +						&child_node_c))
> +					return 1;
> +
> +				if (!(child_node_c != NULL &&
> +					child_node_c !=
> +					node_c->ptrs[n].ptr))
> +					continue;
> +
> +				node_b_refs++;
> +
> +				/*
> +				 * Added link from C to
> +				 * child_C for all transitions
> +				 * in the intersection.
> +				 */
> +				acl_add_ptr(context, node_c,
> +					child_node_c,
> +					&child_intersect);
> +
> +				/*
> +				 * inc refs if pointer is not
> +				 * to node b.
> +				 */
> +				node_a_refs += (child_node_c !=
> +						node_b->ptrs[m].ptr);
> +
> +				/*
> +				 * Remove intersection from C
> +				 * pointer.
> +				 */
> +				if (!acl_exclude(
> +					&node_c->ptrs[n].values,
> +					&node_c->ptrs[n].values,
> +					&child_intersect)) {
> +					acl_deref_ptr(context,
> +						node_c, n);
> +					node_c->ptrs[n].ptr = NULL;
> +					node_a_refs--;
>  				}
>  			}
>  		}
> @@ -1419,9 +1424,11 @@ build_trie(struct acl_build_context *context, struct rte_acl_build_rule *head,
>  		 * Setup the results for this rule.
>  		 * The result and priority of each category.
>  		 */
> -		if (end->mrt == NULL &&
> -				(end->mrt = acl_build_alloc(context, 1,
> -				sizeof(*end->mrt))) == NULL)
> +		if (end->mrt == NULL)
> +			end->mrt = acl_build_alloc(context, 1,
> +					sizeof(*end->mrt));
> +
> +		if (end->mrt == NULL)
>  			return NULL;
>  
>  		for (m = 0; m < context->cfg.num_categories; m++) {
> @@ -1806,6 +1813,7 @@ acl_build_tries(struct acl_build_context *context,
>  			next = rule->next;
>  			for (m = 0; m < config->num_fields; m++) {
>  				int x = config->defs[m].field_index;
> +
>  				if (rule->wildness[x] < wild_limit[m]) {
>  					move = 0;
>  					break;
> @@ -1983,20 +1991,24 @@ rte_acl_build(struct rte_acl_ctx *ctx, const struct rte_acl_config *cfg)
>  		rc = -EINVAL;
>  
>  	/* build internal trie representation. */
> -	} else if ((rc = acl_build_tries(&bcx, bcx.build_rules)) == 0) {
> +	} else {
> +		rc = acl_build_tries(&bcx, bcx.build_rules);
>  
> -		/* allocate and fill run-time  structures. */
> -		rc = rte_acl_gen(ctx, bcx.tries, bcx.bld_tries,
> +		if (rc == 0) {
> +
> +			/* allocate and fill run-time  structures. */
> +			rc = rte_acl_gen(ctx, bcx.tries, bcx.bld_tries,
>  				bcx.num_tries, bcx.cfg.num_categories,
>  				RTE_ACL_IPV4VLAN_NUM * RTE_DIM(bcx.tries),
>  				bcx.num_build_rules);
> -		if (rc == 0) {
> +			if (rc == 0) {
>  
> -			/* set data indexes. */
> -			acl_set_data_indexes(ctx);
> +				/* set data indexes. */
> +				acl_set_data_indexes(ctx);
>  
> -			/* copy in build config. */
> -			ctx->config = *cfg;
> +				/* copy in build config. */
> +				ctx->config = *cfg;
> +			}
>  		}
>  	}
>  
> diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> index 547e6da..6cd0ca9 100644
> --- a/lib/librte_acl/rte_acl.c
> +++ b/lib/librte_acl/rte_acl.c
> @@ -203,7 +203,8 @@ rte_acl_create(const struct rte_acl_param *param)
>  			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,
> diff --git a/lib/librte_acl/rte_acl_osdep_alone.h b/lib/librte_acl/rte_acl_osdep_alone.h
> index a84b6f9..c70dfb0 100644
> --- a/lib/librte_acl/rte_acl_osdep_alone.h
> +++ b/lib/librte_acl/rte_acl_osdep_alone.h
> @@ -186,7 +186,8 @@ rte_rdtsc(void)
>  /**
>   * Force alignment to cache line.
>   */
> -#define	__rte_cache_aligned	__attribute__((__aligned__(RTE_CACHE_LINE_SIZE)))
> +#define	__rte_cache_aligned
> +		__attribute__((__aligned__(RTE_CACHE_LINE_SIZE)))
>  
>  
>  /*
> diff --git a/lib/librte_acl/tb_mem.c b/lib/librte_acl/tb_mem.c
> index fdf3080..eba1723 100644
> --- a/lib/librte_acl/tb_mem.c
> +++ b/lib/librte_acl/tb_mem.c
> @@ -49,8 +49,9 @@ tb_pool(struct tb_mem_pool *pool, size_t sz)
>  	size = sz + pool->alignment - 1;
>  	block = calloc(1, size + sizeof(*pool->block));
>  	if (block == NULL) {
> -		RTE_LOG(ERR, MALLOC, "%s(%zu)\n failed, currently allocated "
> -			"by pool: %zu bytes\n", __func__, sz, pool->alloc);
> +		RTE_LOG(ERR, MALLOC,
> +		"%s(%zu)\n failed, currently allocated by pool: %zu bytes\n",
> +		 __func__, sz, pool->alloc);
>  		return NULL;
>  	}
>  
> -- 
> 1.9.1
>
diff mbox

Patch

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index d6e0c45..1f60411 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -773,11 +773,13 @@  acl_merge(struct acl_build_context *context,
 		for (n = 0; n < ptrs_a; n++) {
 			for (m = 0; m < ptrs_b; m++) {
 
+				uint32_t acl_intsct_type, num_cats;
+
 				if (node_a->ptrs[n].ptr == NULL ||
-						node_b->ptrs[m].ptr == NULL ||
-						node_a->ptrs[n].ptr ==
-						node_b->ptrs[m].ptr)
-						continue;
+					node_b->ptrs[m].ptr == NULL ||
+					node_a->ptrs[n].ptr ==
+					node_b->ptrs[m].ptr)
+					continue;
 
 				intersect_type = acl_intersect_type(
 					&node_a->ptrs[n].values,
@@ -785,35 +787,38 @@  acl_merge(struct acl_build_context *context,
 					&intersect_ptr);
 
 				/* If this node is not a 'match' node */
-				if ((intersect_type & ACL_INTERSECT) &&
-					(context->cfg.num_categories != 1 ||
-					!(node_a->ptrs[n].ptr->match_flag))) {
-
-					/*
-					 * next merge is a 'move' pointer,
-					 * if this one is and B is a
-					 * subset of the intersection.
-					 */
-					next_move = move &&
-						(intersect_type &
-						ACL_INTERSECT_B) == 0;
-
-					if (a_subset && b_full) {
-						rc = acl_merge(context,
-							node_a->ptrs[n].ptr,
-							node_b->ptrs[m].ptr,
-							next_move,
-							1, level + 1);
-						if (rc != 0)
-							return rc;
-					} else {
-						rc = acl_merge_intersect(
-							context, node_a, n,
-							node_b, m, next_move,
-							level, &intersect_ptr);
-						if (rc != 0)
-							return rc;
-					}
+				acl_intsct_type =
+					intersect_type & ACL_INTERSECT;
+				num_cats = (context->cfg.num_categories != 1 ||
+					!(node_a->ptrs[n].ptr->match_flag));
+
+				if (!(acl_intsct_type && num_cats))
+					continue;
+
+				/*
+				 * next merge is a 'move' pointer,
+				 * if this one is and B is a
+				 * subset of the intersection.
+				 */
+				next_move = move &&
+					(intersect_type &
+					ACL_INTERSECT_B) == 0;
+
+				if (a_subset && b_full) {
+					rc = acl_merge(context,
+						node_a->ptrs[n].ptr,
+						node_b->ptrs[m].ptr,
+						next_move,
+						1, level + 1);
+					if (rc != 0)
+						return rc;
+				} else {
+					rc = acl_merge_intersect(
+						context, node_a, n,
+						node_b, m, next_move,
+						level, &intersect_ptr);
+					if (rc != 0)
+						return rc;
 				}
 			}
 		}
@@ -1099,52 +1104,52 @@  acl_merge_trie(struct acl_build_context *context,
 					&node_b->ptrs[m].values,
 					&child_intersect);
 
-				if ((child_intersect_type & ACL_INTERSECT) !=
-						0) {
-					if (acl_merge_trie(context,
-							node_c->ptrs[n].ptr,
-							node_b->ptrs[m].ptr,
-							level + 1, subtree_id,
-							&child_node_c))
-						return 1;
-
-					if (child_node_c != NULL &&
-							child_node_c !=
-							node_c->ptrs[n].ptr) {
-
-						node_b_refs++;
-
-						/*
-						 * Added link from C to
-						 * child_C for all transitions
-						 * in the intersection.
-						 */
-						acl_add_ptr(context, node_c,
-							child_node_c,
-							&child_intersect);
-
-						/*
-						 * inc refs if pointer is not
-						 * to node b.
-						 */
-						node_a_refs += (child_node_c !=
-							node_b->ptrs[m].ptr);
-
-						/*
-						 * Remove intersection from C
-						 * pointer.
-						 */
-						if (!acl_exclude(
-							&node_c->ptrs[n].values,
-							&node_c->ptrs[n].values,
-							&child_intersect)) {
-							acl_deref_ptr(context,
-								node_c, n);
-							node_c->ptrs[n].ptr =
-								NULL;
-							node_a_refs--;
-						}
-					}
+				if ((child_intersect_type & ACL_INTERSECT) ==
+						0)
+					continue;
+
+				if (acl_merge_trie(context,
+						node_c->ptrs[n].ptr,
+						node_b->ptrs[m].ptr,
+						level + 1, subtree_id,
+						&child_node_c))
+					return 1;
+
+				if (!(child_node_c != NULL &&
+					child_node_c !=
+					node_c->ptrs[n].ptr))
+					continue;
+
+				node_b_refs++;
+
+				/*
+				 * Added link from C to
+				 * child_C for all transitions
+				 * in the intersection.
+				 */
+				acl_add_ptr(context, node_c,
+					child_node_c,
+					&child_intersect);
+
+				/*
+				 * inc refs if pointer is not
+				 * to node b.
+				 */
+				node_a_refs += (child_node_c !=
+						node_b->ptrs[m].ptr);
+
+				/*
+				 * Remove intersection from C
+				 * pointer.
+				 */
+				if (!acl_exclude(
+					&node_c->ptrs[n].values,
+					&node_c->ptrs[n].values,
+					&child_intersect)) {
+					acl_deref_ptr(context,
+						node_c, n);
+					node_c->ptrs[n].ptr = NULL;
+					node_a_refs--;
 				}
 			}
 		}
@@ -1419,9 +1424,11 @@  build_trie(struct acl_build_context *context, struct rte_acl_build_rule *head,
 		 * Setup the results for this rule.
 		 * The result and priority of each category.
 		 */
-		if (end->mrt == NULL &&
-				(end->mrt = acl_build_alloc(context, 1,
-				sizeof(*end->mrt))) == NULL)
+		if (end->mrt == NULL)
+			end->mrt = acl_build_alloc(context, 1,
+					sizeof(*end->mrt));
+
+		if (end->mrt == NULL)
 			return NULL;
 
 		for (m = 0; m < context->cfg.num_categories; m++) {
@@ -1806,6 +1813,7 @@  acl_build_tries(struct acl_build_context *context,
 			next = rule->next;
 			for (m = 0; m < config->num_fields; m++) {
 				int x = config->defs[m].field_index;
+
 				if (rule->wildness[x] < wild_limit[m]) {
 					move = 0;
 					break;
@@ -1983,20 +1991,24 @@  rte_acl_build(struct rte_acl_ctx *ctx, const struct rte_acl_config *cfg)
 		rc = -EINVAL;
 
 	/* build internal trie representation. */
-	} else if ((rc = acl_build_tries(&bcx, bcx.build_rules)) == 0) {
+	} else {
+		rc = acl_build_tries(&bcx, bcx.build_rules);
 
-		/* allocate and fill run-time  structures. */
-		rc = rte_acl_gen(ctx, bcx.tries, bcx.bld_tries,
+		if (rc == 0) {
+
+			/* allocate and fill run-time  structures. */
+			rc = rte_acl_gen(ctx, bcx.tries, bcx.bld_tries,
 				bcx.num_tries, bcx.cfg.num_categories,
 				RTE_ACL_IPV4VLAN_NUM * RTE_DIM(bcx.tries),
 				bcx.num_build_rules);
-		if (rc == 0) {
+			if (rc == 0) {
 
-			/* set data indexes. */
-			acl_set_data_indexes(ctx);
+				/* set data indexes. */
+				acl_set_data_indexes(ctx);
 
-			/* copy in build config. */
-			ctx->config = *cfg;
+				/* copy in build config. */
+				ctx->config = *cfg;
+			}
 		}
 	}
 
diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
index 547e6da..6cd0ca9 100644
--- a/lib/librte_acl/rte_acl.c
+++ b/lib/librte_acl/rte_acl.c
@@ -203,7 +203,8 @@  rte_acl_create(const struct rte_acl_param *param)
 			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,
diff --git a/lib/librte_acl/rte_acl_osdep_alone.h b/lib/librte_acl/rte_acl_osdep_alone.h
index a84b6f9..c70dfb0 100644
--- a/lib/librte_acl/rte_acl_osdep_alone.h
+++ b/lib/librte_acl/rte_acl_osdep_alone.h
@@ -186,7 +186,8 @@  rte_rdtsc(void)
 /**
  * Force alignment to cache line.
  */
-#define	__rte_cache_aligned	__attribute__((__aligned__(RTE_CACHE_LINE_SIZE)))
+#define	__rte_cache_aligned
+		__attribute__((__aligned__(RTE_CACHE_LINE_SIZE)))
 
 
 /*
diff --git a/lib/librte_acl/tb_mem.c b/lib/librte_acl/tb_mem.c
index fdf3080..eba1723 100644
--- a/lib/librte_acl/tb_mem.c
+++ b/lib/librte_acl/tb_mem.c
@@ -49,8 +49,9 @@  tb_pool(struct tb_mem_pool *pool, size_t sz)
 	size = sz + pool->alignment - 1;
 	block = calloc(1, size + sizeof(*pool->block));
 	if (block == NULL) {
-		RTE_LOG(ERR, MALLOC, "%s(%zu)\n failed, currently allocated "
-			"by pool: %zu bytes\n", __func__, sz, pool->alloc);
+		RTE_LOG(ERR, MALLOC,
+		"%s(%zu)\n failed, currently allocated by pool: %zu bytes\n",
+		 __func__, sz, pool->alloc);
 		return NULL;
 	}