Re: [RFC PATCH 1/1] checkpatch: Add warning for msleep with durations suitable for ssleep

From: Joe Perches
Date: Sat Mar 09 2024 - 14:47:25 EST


On Tue, 2024-03-05 at 23:47 +0800, Li Chen wrote:
> From: Li Chen <chenl311@xxxxxxxxxxxxxxx>
[]
> Warn when msleep(x000); can be replaced with ssleep(x);

While I don't really see the point as msleep is trivial
to read and ssleep is just msleep * 1000 and there are
already 3:1 more msleep(n*1000) uses than there are ssleep(n)

$ git grep -P '\bmsleep\s*\(\s*\d+000\s*\)' | wc -l
267
$ git grep -P '\bssleep\s*\(\s*\d+\s*\)' | wc -l
89

And about the patch itself:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -6599,6 +6599,16 @@ sub process {
> }
> }
>
> +# warn about msleep() calls with durations that should use ssleep()
> +if ($line =~ /\bmsleep\s*\((\d+)\);/) {

This indentation is incorrect.
And this would be more sensible using
if ($line =~ /\bmsleep\s*\(\s*(\d+)000\s*\)/) {

to avoid the extra tests below

> + my $ms_duration = $1;
> + if ($ms_duration >= 1000 && ($ms_duration % 1000) == 0) {
> + my $ss_duration = $ms_duration / 1000;
> + WARN("SSLEEP",
> + "Prefer ssleep($ss_duration) over msleep($ms_duration);\n" $herecurr);

And please add a $fix to this so:

if ($line =~ /\bmsleep\s*\(\s*(\d+)000\s*\)/) {
$secs = $1;
if (WARN("SSLEEP",
"Prefer ssleep($secs) over msleep(${secs}000\n") &&
$fix) {
$fixed[$fixlinenr] =~ s/\bmsleep\s*\(\s*${secs}000\s*\)/ssleep($secs)/;
}