diff options
| author | Sean Anderson | 2021-03-11 00:15:45 -0500 |
|---|---|---|
| committer | Tom Rini | 2021-04-12 17:44:55 -0400 |
| commit | d9c305071069164feceaff3e543c61693bc2c401 (patch) | |
| tree | ba458cbfbe3747fd097207451e9cf543ca7f2cfd /scripts | |
| parent | 69a752983171c2983fc1f29c7cfa1844f41e5d8b (diff) | |
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 <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Diffstat (limited to 'scripts')
| -rwxr-xr-x | scripts/checkpatch.pl | 6 |
1 files changed, 6 insertions, 0 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 755f4802a46..4e047586a68 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", |