[dpdk-dev] [PATCH v2] devtools: forbid variable declaration inside for
David Marchand
david.marchand at redhat.com
Thu May 28 14:30:24 CEST 2020
On Sun, May 24, 2020 at 7:30 PM Thomas Monjalon <thomas at monjalon.net> wrote:
>
> Some compilers raise an error when declaring a variable
> in the middle of a function. This is a C99 allowance.
> Even if DPDK switches globally to C99 or C11 standard,
> the coding rules are for declarations at the beginning
> of a block:
> http://doc.dpdk.org/guides/contributing/coding_style.html#local-variables
>
> This coding style is enforced by adding a check of
> the common patterns like "for (int i;"
>
> The occurrences of the checked pattern are fixed:
> 'for *(\(char\|u\?int\|unsigned\|s\?size_t\)'
> In the file dpaa2_sparser.c, the fix is to remove the unused macros.
>
> Signed-off-by: Thomas Monjalon <thomas at monjalon.net>
> ---
> v2:
> - check s?size_t and fix one occurence
> - fix new occurences in Windows EAL
> - test-fib has been fixed in the meantime
> ---
> devtools/checkpatches.sh | 8 ++++++++
> doc/guides/prog_guide/eventdev.rst | 6 ++++--
> drivers/common/mlx5/mlx5_devx_cmds.c | 4 ++--
> drivers/common/mlx5/mlx5_glue.c | 3 ++-
> drivers/crypto/caam_jr/caam_jr.c | 5 ++++-
> drivers/net/dpaa2/dpaa2_sparser.c | 30 ----------------------------
> lib/librte_eal/windows/eal_lcore.c | 6 ++++--
> 7 files changed, 24 insertions(+), 38 deletions(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 42b833e0d7..5763a7e953 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -69,6 +69,14 @@ check_forbidden_additions() { # <patch>
> -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> "$1" || res=1
>
> + # forbid variable declaration inside "for" loop
> + awk -v FOLDERS='.' \
> + -v EXPRESSIONS='for *\\((char|u?int|unsigned|s?size_t)' \
> + -v RET_ON_FAIL=1 \
> + -v MESSAGE='Declaring a variable inside for()' \
> + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
> + "$1" || res=1
> +
> # svg figures must be included with wildcard extension
> # because of png conversion for pdf docs
> awk -v FOLDERS='doc' \
> diff --git a/doc/guides/prog_guide/eventdev.rst b/doc/guides/prog_guide/eventdev.rst
> index 7bcd7603b1..ccde086f63 100644
> --- a/doc/guides/prog_guide/eventdev.rst
> +++ b/doc/guides/prog_guide/eventdev.rst
> @@ -242,9 +242,10 @@ Once queues are set up successfully, create the ports as required.
> };
> int dev_id = 0;
> int rx_port_id = 0;
> + int worker_port_id;
> int err = rte_event_port_setup(dev_id, rx_port_id, &rx_conf);
>
> - for(int worker_port_id = 1; worker_port_id <= 4; worker_port_id++) {
> + for (worker_port_id = 1; worker_port_id <= 4; worker_port_id++) {
> int err = rte_event_port_setup(dev_id, worker_port_id, &worker_conf);
> }
>
> @@ -277,8 +278,9 @@ can be achieved like this:
> uint8_t atomic_qs[] = {0, 1};
> uint8_t single_link_q = 2;
> uint8_t priority = RTE_EVENT_DEV_PRIORITY_NORMAL;
> + int worker_port_id;
>
> - for(int worker_port_id = 1; worker_port_id <= 4; worker_port_id++) {
> + for (worker_port_id = 1; worker_port_id <= 4; worker_port_id++) {
> int links_made = rte_event_port_link(dev_id, worker_port_id, atomic_qs, NULL, 2);
> }
> int links_made = rte_event_port_link(dev_id, tx_port_id, &single_link_q, &priority, 1);
> diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c b/drivers/common/mlx5/mlx5_devx_cmds.c
> index fba485e724..1c8b36c91a 100644
> --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> @@ -416,7 +416,7 @@ mlx5_devx_cmd_query_hca_attr(void *ctx,
> uint32_t in[MLX5_ST_SZ_DW(query_hca_cap_in)] = {0};
> uint32_t out[MLX5_ST_SZ_DW(query_hca_cap_out)] = {0};
> void *hcattr;
> - int status, syndrome, rc;
> + int status, syndrome, rc, i;
>
> MLX5_SET(query_hca_cap_in, in, opcode, MLX5_CMD_OP_QUERY_HCA_CAP);
> MLX5_SET(query_hca_cap_in, in, op_mod,
> @@ -529,7 +529,7 @@ mlx5_devx_cmd_query_hca_attr(void *ctx,
> attr->lro_max_msg_sz_mode = MLX5_GET
> (per_protocol_networking_offload_caps,
> hcattr, lro_max_msg_sz_mode);
> - for (int i = 0 ; i < MLX5_LRO_NUM_SUPP_PERIODS ; i++) {
> + for (i = 0 ; i < MLX5_LRO_NUM_SUPP_PERIODS ; i++) {
> attr->lro_timer_supported_periods[i] =
> MLX5_GET(per_protocol_networking_offload_caps, hcattr,
> lro_timer_supported_periods[i]);
> diff --git a/drivers/common/mlx5/mlx5_glue.c b/drivers/common/mlx5/mlx5_glue.c
> index f270f677b7..563941c2df 100644
> --- a/drivers/common/mlx5/mlx5_glue.c
> +++ b/drivers/common/mlx5/mlx5_glue.c
> @@ -587,11 +587,12 @@ mlx5_glue_dv_create_flow(void *matcher,
> return mlx5dv_dr_rule_create(matcher, match_value, num_actions,
> (struct mlx5dv_dr_action **)actions);
> #else
> + size_t i;
> struct mlx5dv_flow_action_attr actions_attr[8];
>
> if (num_actions > 8)
> return NULL;
> - for (size_t i = 0; i < num_actions; i++)
> + for (i = 0; i < num_actions; i++)
> actions_attr[i] =
> *((struct mlx5dv_flow_action_attr *)(actions[i]));
> return mlx5dv_create_flow(matcher, match_value,
> diff --git a/drivers/crypto/caam_jr/caam_jr.c b/drivers/crypto/caam_jr/caam_jr.c
> index caf2386772..77fa6ff2d3 100644
> --- a/drivers/crypto/caam_jr/caam_jr.c
> +++ b/drivers/crypto/caam_jr/caam_jr.c
> @@ -1351,6 +1351,9 @@ caam_jr_enqueue_op(struct rte_crypto_op *op, struct caam_jr_qp *qp)
> struct caam_jr_session *ses;
> struct caam_jr_op_ctx *ctx = NULL;
> struct sec_job_descriptor_t *jobdescr __rte_unused;
> +#if CAAM_JR_DBG
> + int i;
> +#endif
>
> switch (op->sess_type) {
> case RTE_CRYPTO_OP_WITH_SESSION:
> @@ -1413,7 +1416,7 @@ caam_jr_enqueue_op(struct rte_crypto_op *op, struct caam_jr_qp *qp)
> rte_pktmbuf_data_len(op->sym->m_src));
>
> printf("\n JD before conversion\n");
> - for (int i = 0; i < 12; i++)
> + for (i = 0; i < 12; i++)
> printf("\n 0x%08x", ctx->jobdes.desc[i]);
> #endif
>
> diff --git a/drivers/net/dpaa2/dpaa2_sparser.c b/drivers/net/dpaa2/dpaa2_sparser.c
> index 7e8fedd818..ba0d500f74 100644
> --- a/drivers/net/dpaa2/dpaa2_sparser.c
> +++ b/drivers/net/dpaa2/dpaa2_sparser.c
> @@ -145,36 +145,6 @@ struct frame_attr_ext frame_attr_ext_arr[] = {
> /* 112 */ {NULL, 0, 0x0000}
> };
>
> -#define SWAP_WORD(pr) \
> -do { \
> - for (int i = 0; i < 4 ; i++) { \
> - pr[i] = pr[i] ^ pr[6 - i + 1]; \
> - pr[6 - i + 1] = pr[6 - i + 1] ^ pr[i]; \
> - pr[i] = pr[i] ^ pr[6 - i + 1]; \
> - } \
> -} while (0)
> -
> -#define fa_print_sb() \
> -do { \
> - if (rte_cpu_to_be_32(*pdw) & frm_attr->fld_mask) \
> - DPAA2_PMD_DP_DEBUG("t %s : Yes", frm_attr->fld_name); \
> -} while (0)
> -
> -#define fa_print_sb_ext() \
> -do { \
> - if (rte_cpu_to_be_16(*pw) & frm_attr_ext->fld_mask) \
> - DPAA2_PMD_DP_DEBUG("\t %s : Yes", \
> - frm_attr_ext->fld_name); \
> -} while (0)
> -
> -#define fa_print_mb_ext() \
> -do { \
> - if (rte_cpu_to_be_16(*pw) & frm_attr_ext->fld_mask) \
> - DPAA2_PMD_DP_DEBUG("\t %s : 0x%02x", \
> - frm_attr_ext->fld_name, \
> - rte_cpu_to_be_16(*pw) & frm_attr_ext->fld_mask);\
> -} while (0)
> -
> int dpaa2_eth_load_wriop_soft_parser(struct dpaa2_dev_priv *priv,
> enum dpni_soft_sequence_dest dest)
> {
> diff --git a/lib/librte_eal/windows/eal_lcore.c b/lib/librte_eal/windows/eal_lcore.c
> index 82ee454134..760b3fcfed 100644
> --- a/lib/librte_eal/windows/eal_lcore.c
> +++ b/lib/librte_eal/windows/eal_lcore.c
> @@ -29,6 +29,8 @@ static struct _wcpu_map {
> void
> eal_create_cpu_map()
> {
> + unsigned int socket, core;
> +
> wcpu_map.total_procs =
> GetActiveProcessorCount(ALL_PROCESSOR_GROUPS);
>
> @@ -56,9 +58,9 @@ eal_create_cpu_map()
> * equally across the sockets.
> */
> unsigned int lcore = 0;
> - for (unsigned int socket = 0; socket <
> + for (socket = 0; socket <
> wcpu_map.proc_sockets; ++socket) {
> - for (unsigned int core = 0;
> + for (core = 0;
> core < (wcpu_map.proc_cores / wcpu_map.proc_sockets);
> ++core) {
> wcpu_map.wlcore_map[lcore]
> --
> 2.26.2
>
Acked-by: David Marchand <david.marchand at redhat.com>
--
David Marchand
More information about the dev
mailing list