From: Bruce Richardson <bruce.richardson@intel.com>
To: Ravi Kerur <rkerur@gmail.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] Fix checkpatch errors in librte_acl
Date: Mon, 5 Jan 2015 12:08:49 +0000 [thread overview]
Message-ID: <20150105120849.GE13152@bricha3-MOBL3> (raw)
In-Reply-To: <1419521508-31883-2-git-send-email-rkerur@gmail.com>
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
>
next prev parent reply other threads:[~2015-01-05 12:08 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-25 15:31 [dpdk-dev] [PATCH 0/2] Fix checkpatch errors Ravi Kerur
2014-12-25 15:31 ` [dpdk-dev] [PATCH 1/2] Fix checkpatch errors in librte_acl Ravi Kerur
2015-01-05 12:08 ` Bruce Richardson [this message]
2014-12-25 15:31 ` [dpdk-dev] [PATCH 2/2] Fix checkpatch errors in librte_mempool Ravi Kerur
2015-01-05 12:11 ` [dpdk-dev] [PATCH 0/2] Fix checkpatch errors Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150105120849.GE13152@bricha3-MOBL3 \
--to=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=rkerur@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).