Skip to content

Commit 6f676cd

Browse files
committed
Close the CodeQL alerts in the release check and the governor tool
py/file-not-closed (16): every open(...).read() and json.load(open(...)) in tests/release now goes through read_text/read_json in common.py, or local helpers in gate.py (which stays independent of the suite), all inside a with. py/empty-except (2): the reason written next to each pass. py/unused-import and py/unused-local-variable: removed, with one more unused variable in checks_exec.py before it was flagged. Checked: gate.py still passes 26.09.45, and the suite on the x64 VM gives the same 187 passed, 0 failed. The change to skillfish-cpu-governor is a comment only; it ships with the next release.
1 parent edb38cf commit 6f676cd

6 files changed

Lines changed: 54 additions & 23 deletions

File tree

‎system/usr/local/bin/skillfish-cpu-governor‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ def main(argv):
145145
try:
146146
os.unlink(CONF)
147147
except FileNotFoundError:
148+
# nothing kept already: clearing is done
148149
pass
149150
print(json.dumps(status()))
150151
return 0

‎tests/release/gate.py‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,32 @@
2020
import sys
2121

2222
HERE = os.path.dirname(os.path.abspath(__file__))
23+
24+
25+
def _lines(path):
26+
with open(path, encoding="utf-8") as f:
27+
return f.read().splitlines()
28+
29+
30+
def _json(path):
31+
with open(path, encoding="utf-8") as f:
32+
return json.load(f)
33+
34+
35+
def _sha256(path):
36+
h = hashlib.sha256()
37+
with open(path, "rb") as f:
38+
for block in iter(lambda: f.read(1 << 20), b""):
39+
h.update(block)
40+
return h.hexdigest()
41+
42+
2343
REPORTS = os.path.expanduser("~/release-reports")
2444

2545

2646
def kinds():
2747
out = []
28-
for line in open(os.path.join(HERE, "hosts.conf")):
48+
for line in _lines(os.path.join(HERE, "hosts.conf")):
2949
line = line.strip()
3050
if line and not line.startswith("#"):
3151
out.append(line.split()[0])
@@ -44,20 +64,20 @@ def main(debs):
4464
d = os.path.join(REPORTS, ver or "?")
4565
tested = {}
4666
try:
47-
for line in open(os.path.join(d, "debs.sha256")):
67+
for line in _lines(os.path.join(d, "debs.sha256")):
4868
h, name = line.split()
4969
tested[name] = h
5070
except OSError:
5171
# no fingerprint list: this version was never checked
5272
pass
53-
h = hashlib.sha256(open(deb, "rb").read()).hexdigest()
73+
h = _sha256(deb)
5474
if tested.get(os.path.basename(deb)) != h:
5575
# an upstream-versioned package (vkpeak, Mesa...) is tested inside
5676
# an app release: look for it in any report directory
5777
found = False
5878
for other in os.listdir(REPORTS) if os.path.isdir(REPORTS) else []:
5979
try:
60-
lines = open(os.path.join(REPORTS, other, "debs.sha256")).read().split("\n")
80+
lines = _lines(os.path.join(REPORTS, other, "debs.sha256"))
6181
except OSError:
6282
continue
6383
if "%s %s" % (h, os.path.basename(deb)) in lines:
@@ -68,7 +88,7 @@ def main(debs):
6888
continue
6989
for k in kinds():
7090
try:
71-
r = json.load(open(os.path.join(d, "%s.json" % k)))
91+
r = _json(os.path.join(d, "%s.json" % k))
7292
except (OSError, ValueError):
7393
problems.append("%s: no %s report in %s" % (os.path.basename(deb), k, d))
7494
continue

‎tests/release/suite/checks_exec.py‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import stat
66

77
import coverage
8-
from common import Skip, check, files_of, our_packages, sh
8+
from common import Skip, check, files_of, our_packages, read_text, sh
99

1010

1111
def shipped_executables():
@@ -42,7 +42,7 @@ def static_check(path):
4242
return "binary, libraries resolve"
4343
first = head.split(b"\n", 1)[0]
4444
if b"python" in first:
45-
src = open(path, encoding="utf-8", errors="replace").read()
45+
src = read_text(path)
4646
compile(src, path, "exec")
4747
return "python compiles"
4848
if first.startswith(b"#!") and (b"sh" in first):
@@ -87,7 +87,6 @@ def probe(ctx, path, entry):
8787

8888
def run(ctx):
8989
exes = shipped_executables()
90-
check_names = set()
9190
for pkg, path in exes:
9291
name = "exec %s" % path
9392
# Shared libraries carry the exec bit by convention; they are not
@@ -99,7 +98,6 @@ def run(ctx):
9998
if entry is None:
10099
ctx.add(name, "fail", "shipped by %s but has no entry in coverage.py: say how it is tested" % pkg)
101100
continue
102-
check_names.add(path)
103101
ctx.run(name + " [static]", static_check, path)
104102
if entry[0] in ("run", "unit"):
105103
ctx.run(name, probe, ctx, path, entry)

‎tests/release/suite/checks_helper.py‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import time
2020

2121
import coverage
22-
from common import Skip, check, preserved, sh
22+
from common import Skip, check, preserved, read_json, read_text, sh
2323

2424
HELPER = "/usr/local/bin/skillfish-cc-helper"
2525

@@ -57,7 +57,7 @@ def close(self):
5757

5858
def _const(name):
5959
"""A path constant from the installed helper, so the tests follow it."""
60-
src = open(HELPER, encoding="utf-8").read()
60+
src = read_text(HELPER)
6161
m = re.search(r'^%s\s*=\s*"([^"]+)"' % name, src, re.M)
6262
return m.group(1) if m else None
6363

@@ -74,7 +74,7 @@ def _bc(ctx):
7474

7575
# ---- coverage of the command list --------------------------------------------
7676
def t_command_coverage(ctx):
77-
src = open(HELPER, encoding="utf-8").read()
77+
src = read_text(HELPER)
7878
cmds = sorted(set(re.findall(r'^\s+if c == "([a-z0-9-]+)":', src, re.M)))
7979
check(cmds, "no commands found in %s" % HELPER)
8080
missing = [c for c in cmds if c not in coverage.CC_COMMANDS]
@@ -119,17 +119,17 @@ def t_cores(ctx, h):
119119
def t_smt(ctx, h):
120120
_bc(ctx)
121121
p = "/sys/devices/system/cpu/smt/control"
122-
if not os.path.exists(p) or open(p).read().strip() != "on":
122+
if not os.path.exists(p) or read_text(p).strip() != "on":
123123
raise Skip("SMT not on or not controllable")
124124
n0 = _nproc()
125125
try:
126126
_ok(h("cpu-smt", on=False), "cpu-smt off")
127-
check(open(p).read().strip() == "off", "smt/control is not off")
127+
check(read_text(p).strip() == "off", "smt/control is not off")
128128
n1 = _nproc()
129129
check(n1 == n0 // 2, "SMT off: %d threads, expected %d" % (n1, n0 // 2))
130130
finally:
131131
_ok(h("cpu-smt", on=True), "cpu-smt on")
132-
check(open(p).read().strip() == "on", "smt/control did not come back on")
132+
check(read_text(p).strip() == "on", "smt/control did not come back on")
133133
check(_nproc() == n0, "threads after SMT back on: %d, expected %d" % (_nproc(), n0))
134134
return "SMT off and on: %d, %d, %d threads" % (n0, n1, _nproc())
135135

@@ -168,7 +168,7 @@ def t_gov_set(ctx, h):
168168
g = _gov(ctx, h)
169169
conf = g["conf"]
170170
with preserved(_gov_conf_file()):
171-
r = _ok(h("gov-set", conf=conf, timeout=90), "gov-set")
171+
_ok(h("gov-set", conf=conf, timeout=90), "gov-set")
172172
g2 = _ok(h("gov-get"), "gov-get")
173173
check(g2["conf"].get("curva") == conf.get("curva") and g2["conf"].get("freq_max") == conf.get("freq_max"),
174174
"the curve read back differs from the one written")
@@ -364,8 +364,9 @@ def _govs():
364364
out = {}
365365
for p in glob.glob("/sys/devices/system/cpu/cpu*/cpufreq/scaling_governor"):
366366
try:
367-
out[p] = open(p).read().strip()
367+
out[p] = read_text(p).strip()
368368
except OSError:
369+
# this CPU went offline between the glob and the read: skip it
369370
pass
370371
return out
371372

@@ -400,7 +401,7 @@ def t_cpu_gov(ctx, h):
400401
finally:
401402
_ok(h("cpu-cores-set", cores=[{"core": c["core"], "online": True}]), "core on")
402403
time.sleep(2) # the udev rule runs asynchronously
403-
back = {open("/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor" % x["cpu"]).read().strip()
404+
back = {read_text("/sys/devices/system/cpu/cpu%d/cpufreq/scaling_governor" % x["cpu"]).strip()
404405
for x in c["cpus"]}
405406
check(back == {other}, "core %d came back on %s, not %s: the udev rule did not apply it"
406407
% (c["core"], sorted(back), other))
@@ -468,11 +469,11 @@ def t_fan(ctx, h, azione, path):
468469
helper: accepted, and nothing the user set comes back different."""
469470
if not os.path.exists(path):
470471
raise Skip("%s does not exist here" % path)
471-
before = json.load(open(path))
472+
before = read_json(path)
472473
fand_on = sh("systemctl is-active skillfish-fand.service", 10)[1].strip() == "active"
473474
with preserved(path):
474475
_ok(h("ventola", azione=azione, dati=before, timeout=240), "ventola %s" % azione)
475-
after = json.load(open(path))
476+
after = read_json(path)
476477
diff = _same_where_both(before, after)
477478
check(not diff, "values changed on the way through the helper:\n" + "\n".join(diff[:20]))
478479
# the daemon read the rewritten file: give it the original bytes back

‎tests/release/suite/checks_system.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
"""The installed system as a whole: packages, dpkg and apt, systemd units,
22
AppStream cards and menu entries."""
3-
import glob
43
import os
54
import re
65

7-
from common import Skip, check, files_of, our_packages, sh
6+
from common import Skip, check, files_of, our_packages, read_text, sh
87

98
# Packages that carry an upstream version rather than ours (YY.MM.N).
109
UPSTREAM_VERSIONED = ("skillfish-vkpeak", "skillfish-mesa-gfx1013", "skillfish-vaapi-encoder",
@@ -116,7 +115,7 @@ def t_desktop_entries(ctx):
116115
for f in files_of(p):
117116
if f.endswith(".desktop") and os.path.isfile(f):
118117
n += 1
119-
for line in open(f, encoding="utf-8", errors="replace"):
118+
for line in read_text(f).splitlines():
120119
if line.startswith(("Exec=", "TryExec=")):
121120
prog = line.split("=", 1)[1].strip().split()[0] if line.split("=", 1)[1].strip() else ""
122121
prog = prog.strip('"')

‎tests/release/suite/common.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Small shared pieces of the release check."""
22
import contextlib
3+
import json
34
import os
45
import subprocess
56

@@ -29,6 +30,17 @@ def check(cond, msg):
2930
raise AssertionError(msg)
3031

3132

33+
def read_text(path):
34+
"""A whole file, closed at once (CodeQL py/file-not-closed)."""
35+
with open(path, encoding="utf-8", errors="replace") as f:
36+
return f.read()
37+
38+
39+
def read_json(path):
40+
with open(path, encoding="utf-8") as f:
41+
return json.load(f)
42+
43+
3244
def our_packages():
3345
"""Installed packages that are ours: skillfish-*, skillfishos-*."""
3446
rc, out, _ = sh("dpkg-query -W -f='${db:Status-Abbrev} ${Package} ${Version}\\n'", 30)

0 commit comments

Comments
 (0)