This change particularly targets replacing msleep() with ssleep() for
delays that are clear multiples of 1000ms, enhancing code semantics and
readability.
I'm contemplating a kernel-wide update to conform to this practice,
pending community feedback. While a sed script
(like '/\bmsleep\s*\(\s*([0-9]+000)\s*\);/{s/\bmsleep\s*\(\s*([0-9]+)000\s*\);/ssleep(\1);/g}')
could automate this, I'm cautious of potential implications and seek
your thoughts on both the patch and the idea of a broad codebase update.
Li Chen (1):
checkpatch: Add warning for msleep with durations suitable for ssleep
scripts/checkpatch.pl | 10 ++++++++++
1 file changed, 10 insertions(+)
--
2.44.0
From: Li Chen <[email protected]>
This patch enhances checkpatch.pl by introducing a warning for instances
where msleep() is used with durations that are multiples of 1000
milliseconds. Such durations are more semantically appropriate for
ssleep().
The goal is to encourage developers to use the most fitting sleep function,
improving code readability.
Warn when msleep(x000); can be replaced with ssleep(x);
Signed-off-by: Li Chen <[email protected]>
---
scripts/checkpatch.pl | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c4c4a61bc83..a32df4ead5d4 100755
--- 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+)\);/) {
+ 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);
+ }
+}
+
# check for comparisons of jiffies
if ($line =~ /\bjiffies\s*$Compare|$Compare\s*jiffies\b/) {
WARN("JIFFIES_COMPARISON",
--
2.44.0
On Tue, 2024-03-05 at 23:47 +0800, Li Chen wrote:
> From: Li Chen <[email protected]>
[]
> 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)/;
}