On 3/10/21 9:42 PM, Tom Rini wrote:
On Wed, Mar 10, 2021 at 08:45:27PM -0500, Sean Anderson wrote: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 <sean...@gmail.com> --- scripts/checkpatch.pl | 6 ++++++ tools/patman/test_checkpatch.py | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 755f4802a4..91365a5529 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5892,6 +5892,12 @@ sub process { } }+# 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); + } + # prefer usleep_range over udelay if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) { my $delay = $1;This needs to be in the u-boot specific checks section (starting at around line 2300) as I assume Linux won't really want this. Thanks!
Ah, I didn't realize we had our own section. Also, as it turns out the strlcat in U-Boot was just a stub, so I will add one in the next revision. --Sean
