diff mbox

checkpatch: Flag code that returns a negative number

Message ID 1464967533-14634-1-git-send-email-nm@ti.com
State Superseded
Headers show

Commit Message

Nishanth Menon June 3, 2016, 3:25 p.m. UTC
In some functions, returning a -ve decimal value is actually a valid
return condition when the function is returning a value, however, it
can also be misused for returning an error value that should ideally
be a valid error code defined in include/uapi/asm-generic/errno-base.h
or include/uapi/asm-generic/errno.h

Considering typical newbie error of doing the following:
int fn(void)
{
	/* ... error condition ... */
	return -1;
}

void fn1(void)
{
	/* some code */
	if (fn() < 0) {
		pr_err("Error occurred\n");
		return;
	}
	/* other cases... */
}

Flag this as a check case for developer verification.

Signed-off-by: Nishanth Menon <nm@ti.com>

---
 scripts/checkpatch.pl | 6 ++++++
 1 file changed, 6 insertions(+)

-- 
2.8.0

Comments

Nishanth Menon June 3, 2016, 4:07 p.m. UTC | #1
On 06/03/2016 11:01 AM, Joe Perches wrote:

[...]
> I did more or less the same grep, and that's somewhat true.

> -1 though is very common and doesn't need to be replaced.

OK,

> 

> $ git grep -E "\breturn\s+\-\s*[0-9]+\s*;" * | grep -v "^tools" | grep -vP "return\s*\-1;" | wc -l

> 211

> 

> Looking at some of the specific instances of negative return values

> instead of the line counts though may show otherwise.

> 

> -EFOO errors aren't always better.

At least would'nt be a little more readable than obscure -val?

Would we like -[2-9][0-9]* flagged at all even as a check?

-- 
Regards,
Nishanth Menon
diff mbox

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4904ced676d4..f6fa07fe33a5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4351,6 +4351,12 @@  sub process {
 			}
 		}
 
+# return with a value is not usually a good sign, unless the function is supposed to return a value
+		if (defined($stat) && $stat =~ /^.\s*return\s*-[0-9]+\s*;/s) {
+			CHK("RETURN_NUMBER",
+			    "Suspect error return with a value, If this is error value, refer to include/uapi/asm-generic/errno-base.h  and include/uapi/asm-generic/errno.h\n" . $herecurr);
+		}
+
 # unnecessary return in a void function
 # at end-of-function, with the previous line a single leading tab, then return;
 # and the line before that not a goto label target like "out:"