[v1] devtools: allow variable declaration inside for loop
Checks
Commit Message
Declaring variable inside for loop is not supported via C89 and it was
checked in checkpatch.sh via commit [1].
But as DPDK supported C standard is becoming C99 [2], declaring variable
inside loop can be allowed.
[1]
Commit 43e73483a4b8 ("devtools: forbid variable declaration inside for")
[2]
https://dpdk.org/patch/121912
Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
Cc: Bruce Richardson <bruce.richardson@intel.com>
Cc: David Marchand <david.marchand@redhat.com>
---
devtools/checkpatches.sh | 8 --------
1 file changed, 8 deletions(-)
Comments
On Wed, May 03, 2023 at 10:50:18AM +0100, Ferruh Yigit wrote:
> Declaring variable inside for loop is not supported via C89 and it was
> checked in checkpatch.sh via commit [1]. But as DPDK supported C
> standard is becoming C99 [2], declaring variable inside loop can be
> allowed.
>
> [1] Commit 43e73483a4b8 ("devtools: forbid variable declaration inside
> for")
>
> [2] https://dpdk.org/patch/121912
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com> --- Cc: Bruce
> Richardson <bruce.richardson@intel.com> Cc: David Marchand
> <david.marchand@redhat.com> --- devtools/checkpatches.sh | 8 -------- 1
> file changed, 8 deletions(-)
>
Definite +1 from me for allowing this. However, is the plan still to move
to C99 in this release. I thought we were just going to jump to C11 in
23.11 release? However, I can't see any compilers refusing this if we do
relax things a bit now.
I was thinking that our coding standards doc might need an update for this,
but I don't see the restriction on not doing this documented there, so it
seems no doc change is necessary.
/Bruce
On 5/3/2023 11:02 AM, Bruce Richardson wrote:
> On Wed, May 03, 2023 at 10:50:18AM +0100, Ferruh Yigit wrote:
>> Declaring variable inside for loop is not supported via C89 and it was
>> checked in checkpatch.sh via commit [1]. But as DPDK supported C
>> standard is becoming C99 [2], declaring variable inside loop can be
>> allowed.
>>
>> [1] Commit 43e73483a4b8 ("devtools: forbid variable declaration inside
>> for")
>>
>> [2] https://dpdk.org/patch/121912
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com> --- Cc: Bruce
>> Richardson <bruce.richardson@intel.com> Cc: David Marchand
>> <david.marchand@redhat.com> --- devtools/checkpatches.sh | 8 -------- 1
>> file changed, 8 deletions(-)
>>
>
> Definite +1 from me for allowing this. However, is the plan still to move
> to C99 in this release. I thought we were just going to jump to C11 in
> 23.11 release? However, I can't see any compilers refusing this if we do
> relax things a bit now.
I will update the commit log for target as C99/C11 .
>
> I was thinking that our coding standards doc might need an update for this,
> but I don't see the restriction on not doing this documented there, so it
> seems no doc change is necessary.
>
Commit 43e73483a4b8 refers to following document:
http://doc.dpdk.org/guides/contributing/coding_style.html#local-variables
Which has: "Variables should be declared at the start of a block of code
rather than in the middle."
Although it is comparing between declaring start of a block and middle
of the code, maybe we can update the document to explicitly state that
declaring variable inside for loop is allowed to prevent confusion.
Let me send a new version with documentation update.
@@ -78,14 +78,6 @@ 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[[:space:]]*\\((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
-
# refrain from new additions of 16/32/64 bits rte_atomicNN_xxx()
awk -v FOLDERS="lib drivers app examples" \
-v EXPRESSIONS="rte_atomic[0-9][0-9]_.*\\\(" \