[PATCH] checkpatch: Add warnings for using strn(cat|cpy)

Sean Anderson seanga2 at gmail.com
Thu Mar 11 03:46:09 CET 2021


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 <seanga2 at 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


More information about the U-Boot mailing list