Re: [dpdk-dev] [PATCH v2] devtools: forbid variable declaration inside for

2020-07-03 Thread David Marchand
On Thu, May 28, 2020 at 2:30 PM David Marchand
 wrote:
> On Sun, May 24, 2020 at 7:30 PM Thomas Monjalon  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 
> Acked-by: David Marchand 

Discarded the (now obsolete) change on windows/eal.c and applied.


-- 
David Marchand



Re: [dpdk-dev] [PATCH v2] devtools: forbid variable declaration inside for

2020-05-28 Thread David Marchand
On Sun, May 24, 2020 at 7:30 PM Thomas Monjalon  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 
> ---
> 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() { # 
> -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 *ma

Re: [dpdk-dev] [PATCH v2] devtools: forbid variable declaration inside for

2020-05-24 Thread Stephen Hemminger
On Sun, 24 May 2020 19:30:07 +0200
Thomas Monjalon  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 

Surprised that checkpatch doesn't enforce this already.
Or maybe kernel flags are different.