[v13] devtools: alert on new instances of rte_panic and rte_exit
Checks
Commit Message
This patch adds a new function that is called
per every checked patch,
and alerts for new instances of rte_panic/rte_exit.
The check excludes comments, and alerts in the case
of a positive balance between additions and removals.
Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
Tested-by: Kevin Traynor <ktraynor@redhat.com>
---
v11
- Use Neils tmpfile infrastructure to consume the patches
- Allow different calls to the awk script decide if they want
to fail the check or just warn
v12
- Restored Tested message
V13
- Multiple formatting comments by Thomas
- Refrain from printing blank success string
devtools/checkpatches.sh | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
Comments
27/07/2018 00:10, Arnon Warshavsky:
> + # refrain from new additions of rte_panic() and rte_exit()
> + # under lib and net
> + # multiple folders and expressions are separated by spaces
> + awk -v FOLDERS="lib net drivers" \
I don't know why net is listed as a root directory here?
I am going to apply the patch without the "net" directory. OK?
> + awk -v FOLDERS="lib net drivers" \
>
> I don't know why net is listed as a root directory here?
> I am going to apply the patch without the "net" directory. OK?
>
Yes please. Its not necessarily root rather in any location of the path,
but having added drivers, net is now redundant.
thanks
/Arnon
27/07/2018 00:10, Arnon Warshavsky:
> This patch adds a new function that is called
> per every checked patch,
> and alerts for new instances of rte_panic/rte_exit.
> The check excludes comments, and alerts in the case
> of a positive balance between additions and removals.
>
> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> Tested-by: Kevin Traynor <ktraynor@redhat.com>
Applied, with small adjustments, thanks
On Tue, Jul 31, 2018 at 2:38 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 27/07/2018 00:10, Arnon Warshavsky:
>> This patch adds a new function that is called
>> per every checked patch,
>> and alerts for new instances of rte_panic/rte_exit.
>> The check excludes comments, and alerts in the case
>> of a positive balance between additions and removals.
>>
>> Signed-off-by: Arnon Warshavsky <arnon@qwilt.com>
>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>> Tested-by: Kevin Traynor <ktraynor@redhat.com>
>
> Applied, with small adjustments, thanks
I get some warning when running this on Ubuntu 18.04.
marchand@gribouille:~/git/dpdk$ ./devtools/checkpatches.sh
sanity/0001-mbuf-add-sanity-checks-on-segment-metadata.patch
./devtools/checkpatches.sh: 52: read: Illegal option -d
1/1 valid patch
+ false
+ check_forbidden_additions
+ read -d awk_script
./devtools/checkpatches.sh: 52: read: Illegal option -d
+ awk -v FOLDERS=lib drivers -v EXPRESSIONS=rte_panic\\( rte_exit\\(
-v RET_ON_FAIL=1 -
+ report=
+ [ 0 -ne 0 ]
I get some warning when running this on Ubuntu 18.04.
>
> marchand@gribouille:~/git/dpdk$ ./devtools/checkpatches.sh
> sanity/0001-mbuf-add-sanity-checks-on-segment-metadata.patch
> ./devtools/checkpatches.sh: 52: read: Illegal option -d
>
> Hi,
A patch that fixes that was already sent a few days ago by Andrzej
Ostruszka
http://patches.dpdk.org/patch/44020/
/Arnon
On Mon, Sep 10, 2018 at 8:17 AM, Arnon Warshavsky <arnon@qwilt.com> wrote:
> I get some warning when running this on Ubuntu 18.04.
>>
>>
>> marchand@gribouille:~/git/dpdk$ ./devtools/checkpatches.sh
>> sanity/0001-mbuf-add-sanity-checks-on-segment-metadata.patch
>> ./devtools/checkpatches.sh: 52: read: Illegal option -d
>>
> Hi,
> A patch that fixes that was already sent a few days ago by Andrzej Ostruszka
> http://patches.dpdk.org/patch/44020/
Ok, will test, thanks.
@@ -43,6 +43,91 @@ print_usage () {
END_OF_HELP
}
+
+check_forbidden_additions() {
+ # This awk script receives a list of expressions to monitor
+ # and a list of folders to search these expressions in
+ # - No search is done inside comments
+ # - Both additions and removals of the expressions are checked
+ # A positive balance of additions fails the check
+ read -d '' awk_script << 'EOF'
+ BEGIN {
+ split(FOLDERS,deny_folders," ");
+ split(EXPRESSIONS,deny_expr," ");
+ in_file=0;
+ in_comment=0;
+ count=0;
+ comment_start="/*"
+ comment_end="*/"
+ }
+ # search for add/remove instances in current file
+ # state machine assumes the comments structure is enforced by
+ # checkpatches.pl
+ (in_file) {
+ # comment start
+ if (index($0,comment_start) > 0){
+ in_comment = 1
+ }
+ # non comment code
+ if (in_comment == 0) {
+ for (i in deny_expr) {
+ forbidden_added = "^\+.*" deny_expr[i];
+ forbidden_removed="^-.*" deny_expr[i];
+ current = expressions[deny_expr[i]]
+ if ($0 ~ forbidden_added) {
+ count = count + 1;
+ expressions[deny_expr[i]] = current + 1
+ }
+ if ($0 ~ forbidden_removed) {
+ count = count - 1;
+ expressions[deny_expr[i]] = current - 1
+ }
+ }
+ }
+ # comment end
+ if (index($0,comment_end) > 0) {
+ in_comment = 0
+ }
+ }
+ # switch to next file , check if the balance of add/remove
+ # of previous filehad new additions
+ ($0 ~ "^\+\+\+ b/") {
+ in_file = 0;
+ if (count > 0){
+ exit;
+ }
+ for (i in deny_folders){
+ re = "^\+\+\+ b/" deny_folders[i];
+ if ($0 ~ deny_folders[i]) {
+ in_file = 1
+ last_file = $0
+ }
+ }
+ }
+ END{
+ if (count > 0){
+ print "Warning in " substr(last_file,6) ":"
+ print "are you sure you want to add the following:"
+ for (key in expressions) {
+ if (expressions[key] > 0) {
+ print key
+ }
+ }
+ exit RET_ON_FAIL
+ }
+ }
+EOF
+ # ---------------------------------
+
+ # refrain from new additions of rte_panic() and rte_exit()
+ # under lib and net
+ # multiple folders and expressions are separated by spaces
+ awk -v FOLDERS="lib net drivers" \
+ -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \
+ -v RET_ON_FAIL=1 \
+ "$awk_script" -
+}
+
number=0
quiet=false
verbose=false
@@ -97,6 +182,13 @@ check () { # <patch> <commit> <title>
ret=1
fi
+ ! $verbose || printf '\nChecking forbidden tokens additions:\n'
+ report=$(check_forbidden_additions <"$tmpinput")
+ if [ $? -ne 0 ] ; then
+ printf '%s\n' "$report"
+ ret=1
+ fi
+
clean_tmp_files
[ $ret -eq 0 ] && return 0