[dpdk-dev] [PATCH 1/2] Fix checkpatch errors in librte_acl
Bruce Richardson
bruce.richardson at intel.com
Mon Jan 5 13:08:49 CET 2015
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 at 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
>
More information about the dev
mailing list