On Thu, 2024-04-11 at 22:01 +0200, Christophe JAILLET wrote: > Le 08/04/2024 à 22:53, Justin Stitt a écrit : > > I am going to quote Lee Jones who has been doing some snprintf -> > > scnprintf refactorings: > > > > "There is a general misunderstanding amongst engineers that > > {v}snprintf() returns the length of the data *actually* encoded into the > > destination array. However, as per the C99 standard {v}snprintf() > > really returns the length of the data that *would have been* written if > > there were enough space for it. This misunderstanding has led to > > buffer-overruns in the past. It's generally considered safer to use the > > {v}scnprintf() variants in their place (or even sprintf() in simple > > cases). So let's do that." > > > > To help prevent new instances of snprintf() from popping up, let's add a > > check to checkpatch.pl. > > > > Suggested-by: Finn Thain <fth...@linux-m68k.org> > > Signed-off-by: Justin Stitt <justinst...@google.com> > > --- > > Changes in v4: > > - also check for vsnprintf variant (thanks Bill) > > - Link to v3: > > https://lore.kernel.org/r/20240315-snprintf-checkpatch-v3-1-a451e7664...@google.com > > > > Changes in v3: > > - fix indentation > > - add reference link (https://github.com/KSPP/linux/issues/105) (thanks Joe) > > - Link to v2: > > https://lore.kernel.org/r/20240221-snprintf-checkpatch-v2-1-9baeb59da...@google.com > > > > Changes in v2: > > - Had a vim moment and deleted a character before sending the patch. > > - Replaced the character :) > > - Link to v1: > > https://lore.kernel.org/r/20240221-snprintf-checkpatch-v1-1-3ac5025b5...@google.com > > --- > > From a discussion here [1]. > > > > [1]: > > https://lore.kernel.org/all/0f9c95f9-2c14-eee6-7faf-635880edc...@linux-m68k.org/ > > --- > > scripts/checkpatch.pl | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 9c4c4a61bc83..a0fd681ea837 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -7012,6 +7012,12 @@ sub process { > > "Prefer strscpy, strscpy_pad, or __nonstring over > > strncpy - see: https://github.com/KSPP/linux/issues/90\n" . $herecurr); > > } > > > > +# {v}snprintf uses that should likely be {v}scnprintf > > + if ($line =~ /\b(v|)snprintf\s*\(\s*/) { > > Hi, > > for my understanding, what is the purpose of the 2nd "\s*"? > IMHO, it could be just removed.
It could. # {v}snprintf uses that should likely be {v}scnprintf if ($line =~ /\b((v?)snprintf)\s*\(/) { WARN("SNPRINTF", "Prefer ${2}scnprintf over $1 - see: https://github.com/KSPP/linux/issues/105\n" . $herecurr); } Though I also think it's better to use lore rather than github