From d9c305071069164feceaff3e543c61693bc2c401 Mon Sep 17 00:00:00 2001 From: Sean Anderson Date: Thu, 11 Mar 2021 00:15:45 -0500 Subject: checkpatch: Add warnings for using strn(cat|cpy) strn(cat|cpy) has a bad habit of not nul-terminating the destination, resulting in constructions like strncpy(foo, bar, sizeof(foo) - 1); foo[sizeof(foo) - 1] = '\0'; However, it is very easy to forget about this behavior and accidentally leave a string unterminated. This has shown up in some recent coverity scans [1, 2] (including code recently touched by yours truly). Fortunately, the guys at OpenBSD came up with strl(cat|cpy), which always nul-terminate strings. These functions are already in U-Boot, so we should encourage new code to use them instead of strn(cat|cpy). [1] https://lists.denx.de/pipermail/u-boot/2021-March/442888.html [2] https://lists.denx.de/pipermail/u-boot/2021-January/438073.html Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 755f4802a4..4e047586a6 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2365,6 +2365,12 @@ sub u_boot_line { "Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible\n" . $herecurr); } + # prefer strl(cpy|cat) over strn(cpy|cat) + if ($line =~ /\bstrn(cpy|cat)\s*\(/) { + WARN("STRL", + "strl$1 is preferred over strn$1 because it always produces a nul-terminated string\n" . $herecurr); + } + # use defconfig to manage CONFIG_CMD options if ($line =~ /\+\s*#\s*(define|undef)\s+(CONFIG_CMD\w*)\b/) { ERROR("DEFINE_CONFIG_CMD", -- cgit v1.2.3