Received-SPF: pass (google.com: domain of linux-kernel+bounces-98023-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <0967032faf79111417b2014ccb80077a804859ce.camel@perches.com>
Subject: Re: [RFC PATCH 1/1] checkpatch: Add warning for msleep with
 durations suitable for ssleep
From: Joe Perches <joe@perches.com>
To: Li Chen <me@linux.beauty>, Andy Whitcroft <apw@canonical.com>, Dwaipayan
	Ray <dwaipayanray1@gmail.com>, Lukas Bulwahn <lukas.bulwahn@gmail.com>, 
	linux-kernel@vger.kernel.org
Date: Sat, 09 Mar 2024 11:47:13 -0800
In-Reply-To: <87edcor88b.wl-me@linux.beauty>
References: <87frx4r8dt.wl-me@linux.beauty> <87edcor88b.wl-me@linux.beauty>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
Precedence: bulk
MIME-Version: 1.0

On Tue, 2024-03-05 at 23:47 +0800, Li Chen wrote:
> From: Li Chen <chenl311@chinatelecom.cn>
[]
> 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 {
>  			}
>  		}
> =20
> +# warn about msleep() calls with durations that should use ssleep()
> +if ($line =3D~ /\bmsleep\s*\((\d+)\);/) {

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

to avoid the extra tests below

> +    my $ms_duration =3D $1;
> +    if ($ms_duration >=3D 1000 && ($ms_duration % 1000) =3D=3D 0) {
> +        my $ss_duration =3D $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 =3D~ /\bmsleep\s*\(\s*(\d+)000\s*\)/) {
			$secs =3D $1;
			if (WARN("SSLEEP",
				 "Prefer ssleep($secs) over msleep(${secs}000\n") &&
			    $fix) {
				$fixed[$fixlinenr] =3D~ s/\bmsleep\s*\(\s*${secs}000\s*\)/ssleep($secs)=
/;
			}