From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id A4935593A for ; Mon, 5 Jan 2015 13:08:53 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP; 05 Jan 2015 04:08:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,691,1406617200"; d="scan'208";a="507541296" Received: from bricha3-mobl3.ger.corp.intel.com ([10.243.20.27]) by orsmga003.jf.intel.com with SMTP; 05 Jan 2015 04:03:16 -0800 Received: by (sSMTP sendmail emulation); Mon, 05 Jan 2015 12:08:49 +0025 Date: Mon, 5 Jan 2015 12:08:49 +0000 From: Bruce Richardson To: Ravi Kerur Message-ID: <20150105120849.GE13152@bricha3-MOBL3> References: <1419521508-31883-1-git-send-email-rkerur@gmail.com> <1419521508-31883-2-git-send-email-rkerur@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1419521508-31883-2-git-send-email-rkerur@gmail.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 1/2] Fix checkpatch errors in librte_acl X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Jan 2015 12:08:54 -0000 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 > > 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 > --- > 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 >