From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 302371C01 for ; Thu, 26 Jul 2018 22:30:02 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id B42CD21BFD; Thu, 26 Jul 2018 16:30:01 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Thu, 26 Jul 2018 16:30:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=7T7UaVgL3zLwar31w3MMMT9yWx pryRiJfrVacC5yt9c=; b=pacMFPESfe6tfnlRaTZKEM0tr7BJFK7ZGI6w3oP75Y tLjEsYzRksxybsZnknOdYBlXEBEsCpBWg7xyv0OZS/u4zCp5qFERshpAb6lKuJRV iTnxcGUjAXpOOdlCJ8CJYhaZwXusMkziIfD4r/yJcMasumN++gvEA6XXpVMVs4T6 I= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=7T7UaV gL3zLwar31w3MMMT9yWxpryRiJfrVacC5yt9c=; b=edy8RSDfmtydnwjsJ9pyyr xgLSyfoul8RsIlOq2UnLbuKFzZwL4gJXlMxHkbd3JiitI+4b3MuSGHfkjlU26aTp 1XoNIESxybl2Z5aUPi7+9bGUluF/hIVdWNw4WkWGtVE4njcqdkQ/M1aklMi92AVg /2SwBf5uQYjut0orSG8ma7ugyMzzmKNMwE0N5BzMCKdu0phBs798yQRx+tkMqpNY Hf3KGOGv4M9PNfbdUAim9W5zTONh/KH3C1/+2t/JYpz02rRE8IZL4KmDAKe5wOs9 lMQzvEFYkNZTOT5oJAgGCRlzZpiJLXoRKDD5FKJB5AvA7Q7dbtVSrDsQN+zx/kxw == X-ME-Proxy: X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id D36791025D; Thu, 26 Jul 2018 16:29:59 -0400 (EDT) From: Thomas Monjalon To: Arnon Warshavsky Cc: dev@dpdk.org, anatoly.burakov@intel.com, wenzhuo.lu@intel.com, declan.doherty@intel.com, jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com, ferruh.yigit@intel.com Date: Thu, 26 Jul 2018 22:29:55 +0200 Message-ID: <5378502.1ef9rKBE6e@xps> In-Reply-To: <1531745044-19185-1-git-send-email-arnon@qwilt.com> References: <1531741077-5513-1-git-send-email-arnon@qwilt.com> <1531745044-19185-1-git-send-email-arnon@qwilt.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v12] devtools: alert on new instances of rte_panic and rte_exit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Jul 2018 20:30:02 -0000 Hi, I am sorry, I always give low priority to tooling review. I was going to apply this one, but I've seen some details to improve. 16/07/2018 14:44, Arnon Warshavsky: > +check_forbidden_additions() { # This function looks to work with stdin, not a file. Better to remove the comment about a . > + # --------------------------------- A split line at the beginning is visually strange. > + #This awk script receives a list of expressions to monitor > + #and a list of folders to search these expressions in Please insert a space after # > + # - 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' EOF must be quoted? > + BEGIN{ Please insert a space before { > + 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 > + } > + } > + } > + This is the only blank line in the awk script. Can be removed. > + # 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){ > + warnText = "\\033[1;31m" "Warning:" "\\033[0m" Please no color in the warning. It will look strange in a file. > + print warnText " 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" \ Why not looking in drivers directory? > + -v EXPRESSIONS="rte_panic\\\( rte_exit\\\(" \ > + -v RET_ON_FAIL=0 \ > + "$awk_script" - > +} > + > number=0 > quiet=false > verbose=false > @@ -97,6 +185,13 @@ check () { # > ret=1 > fi > > + ! $verbose || printf '\nChecking forbidden tokens additions/removals:\n' > + report=$(cat $tmpinput | check_forbidden_additions) Another way of writing it without cat: check_forbidden_additions <"tmpinput" In any case, please quote "tmpinput" to prevent from space. > + if [ $? -ne 0 ] ; then > + ret=1 > + fi > + printf '%s\n' "$report" You are printing the report, no matter of the result? Why? Is it because a warning does not return as an error? There is maybe an improvement required here.