diff options
| author | Sean Anderson <seanga2@gmail.com> | 2021-03-11 00:15:45 -0500 |
|---|---|---|
| committer | Tom Rini <trini@konsulko.com> | 2021-04-12 17:44:55 -0400 |
| commit | d9c305071069164feceaff3e543c61693bc2c401 (patch) | |
| tree | ba458cbfbe3747fd097207451e9cf543ca7f2cfd /tools | |
| parent | 69a752983171c2983fc1f29c7cfa1844f41e5d8b (diff) | |
| download | u-boot-d9c305071069164feceaff3e543c61693bc2c401.tar.gz | |
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 'tools')
| -rw-r--r-- | tools/patman/test_checkpatch.py | 14 |
1 files changed, 13 insertions, 1 deletions
diff --git a/tools/patman/test_checkpatch.py b/tools/patman/test_checkpatch.py index a4fec1d4c1..56af5265cc 100644 --- a/tools/patman/test_checkpatch.py +++ b/tools/patman/test_checkpatch.py @@ -353,7 +353,7 @@ index 0000000..2234c87 Args: pm: PatchMaker object to use - msg" Expected message (e.g. 'LIVETREE') + msg: Expected message (e.g. 'LIVETREE') pmtype: Type of problem ('error', 'warning') """ result = pm.run_checkpatch() @@ -439,6 +439,18 @@ index 0000000..2234c87 self.check_struct('per_device_auto', '_priv', 'DEVICE_PRIV_AUTO') self.check_struct('per_device_plat_auto', '_plat', 'DEVICE_PLAT_AUTO') + def check_strl(self, func): + """Check one of the checks for strn(cpy|cat)""" + pm = PatchMaker() + pm.add_line('common/main.c', "strn%s(foo, bar, sizeof(foo));" % func) + self.checkSingleMessage(pm, "STRL", + "strl%s is preferred over strn%s because it always produces a nul-terminated string\n" + % (func, func)) + + def testStrl(self): + """Check for uses of strn(cat|cpy)""" + self.check_strl("cat"); + self.check_strl("cpy"); if __name__ == "__main__": unittest.main() |