| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1553565318-15108-1-git-send-email-hofrat@osadl.org>
Date: Tue, 26 Mar 2019 02:55:18 +0100
From: Nicholas Mc Guire <hofrat@...dl.org>
To: Ben Skeggs <bskeggs@...hat.com>
Cc: David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, Nicholas Mc Guire <hofrat@...dl.org>
Subject: [RFC PATCH] drm/nouveau/fb/ram/gk104: move assignment out of condition
"hiding" unconditional assignments in the if() parentesis makes for
hard to read code and has no advantage over placing these assignments
in proper formated lines before the if() statement. Simply move those
lines out.
Before sending out roughly 20 patches to fix the roughly 50 cases - all
in the nouveau driver. I would like to know if this will be accepted at
all.
Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
---
Problem located by and experiemental coccinelle script.
Note that the last hunk retained the if (i == 0) rather than switching
to a more consistent if (!i) - but then again the code is not really
consistent on that - so not sure if that should be done or not.
Cocciscript to generate the appropriate patches - note though that it
will *not* remove excess parentesis where these are present in the
current code:
<snip>
/// Check for unconditional code "hiding" in an if condition
/// effectively that code is unconditionally executed before
/// reaching the actual branch statement - which just makes it
/// hard to read and thus is always wrong.
/// Some of the cases found also look buggy
/// In other cases some excess () are left in place in the
/// generated patches - so some postprocessing may be needed.
///
/// As of 5.0-rc8 all 50 cases look like they are found and fixed
/// correctly - correctly only in the sense that the patched
/// code is equivalent to the original code. but as this is in
/// the nouveau driver only it might well be that this only
/// fits that specific pattern and others might have wilder ways
/// to achieve the same level of confusing code- so confidence
/// Low for now (though the the nouveau cases all are patched
/// correctly)
///
// Copyright: (C) 2019 Nicholas Mc Guire. GPL-V2.
// Confidence: Low
// Comments:
// Options: --no-includes --include-headers
virtual patch
virtual report
@badif@
position p;
statement S;
expression E1,E2;
@@
if@p (E1,E2) S
@script:python depends on report@
p << badif.p;
@@
msg = "unconditional code hiding in if condition"
coccilib.report.print_report(p[0],msg)
@fixbadif depends on badif && patch@
position p=badif.p;
statement S;
expression E1=badif.E1,E2=badif.E2;
@@
+ E1;
if@p (
- E1,
E2)
S
@script:python depends on patch@
p << fixbadif.p;
@@
<snip>
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c | 42 +++++++++++++++--------
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
index 8bcb7e7..885c24d 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
@@ -1168,7 +1168,8 @@ gk104_ram_prog_0(struct gk104_ram *ram, u32 freq)
if (&cfg->head == &ram->cfg)
return;
- if (mask = 0, data = 0, ram->diff.rammap_11_0a_03fe) {
+ mask = 0, data = 0;
+ if (ram->diff.rammap_11_0a_03fe) {
data |= cfg->bios.rammap_11_0a_03fe << 12;
mask |= 0x001ff000;
}
@@ -1178,31 +1179,36 @@ gk104_ram_prog_0(struct gk104_ram *ram, u32 freq)
}
nvkm_mask(device, 0x10f468, mask, data);
- if (mask = 0, data = 0, ram->diff.rammap_11_0a_0400) {
+ mask = 0, data = 0;
+ if (ram->diff.rammap_11_0a_0400) {
data |= cfg->bios.rammap_11_0a_0400;
mask |= 0x00000001;
}
nvkm_mask(device, 0x10f420, mask, data);
- if (mask = 0, data = 0, ram->diff.rammap_11_0a_0800) {
+ mask = 0, data = 0;
+ if (ram->diff.rammap_11_0a_0800) {
data |= cfg->bios.rammap_11_0a_0800;
mask |= 0x00000001;
}
nvkm_mask(device, 0x10f430, mask, data);
- if (mask = 0, data = 0, ram->diff.rammap_11_0b_01f0) {
+ mask = 0, data = 0;
+ if (ram->diff.rammap_11_0b_01f0) {
data |= cfg->bios.rammap_11_0b_01f0;
mask |= 0x0000001f;
}
nvkm_mask(device, 0x10f400, mask, data);
- if (mask = 0, data = 0, ram->diff.rammap_11_0b_0200) {
+ mask = 0, data = 0;
+ if (ram->diff.rammap_11_0b_0200) {
data |= cfg->bios.rammap_11_0b_0200 << 9;
mask |= 0x00000200;
}
nvkm_mask(device, 0x10f410, mask, data);
- if (mask = 0, data = 0, ram->diff.rammap_11_0d) {
+ mask = 0, data = 0;
+ if (ram->diff.rammap_11_0d) {
data |= cfg->bios.rammap_11_0d << 16;
mask |= 0x00ff0000;
}
@@ -1212,7 +1218,8 @@ gk104_ram_prog_0(struct gk104_ram *ram, u32 freq)
}
nvkm_mask(device, 0x10f440, mask, data);
- if (mask = 0, data = 0, ram->diff.rammap_11_0e) {
+ mask = 0, data = 0;
+ if (ram->diff.rammap_11_0e) {
data |= cfg->bios.rammap_11_0e << 8;
mask |= 0x0000ff00;
}
@@ -1453,17 +1460,21 @@ gk104_ram_ctor_data(struct gk104_ram *ram, u8 ramcfg, int i)
/* memory config data for a range of target frequencies */
data = nvbios_rammapEp(bios, i, &ver, &hdr, &cnt, &len, &cfg->bios);
- if (ret = -ENOENT, !data)
+ ret = -ENOENT;
+ if (!data)
goto done;
- if (ret = -ENOSYS, ver != 0x11 || hdr < 0x12)
+ ret = -ENOSYS;
+ if (ver != 0x11 || hdr < 0x12)
goto done;
/* ... and a portion specific to the attached memory */
data = nvbios_rammapSp(bios, data, ver, hdr, cnt, len, ramcfg,
&ver, &hdr, &cfg->bios);
- if (ret = -EINVAL, !data)
+ ret = -EINVAL;
+ if (!data)
goto done;
- if (ret = -ENOSYS, ver != 0x11 || hdr < 0x0a)
+ ret = -ENOSYS;
+ if (ver != 0x11 || hdr < 0x0a)
goto done;
/* lookup memory timings, if bios says they're present */
@@ -1471,14 +1482,17 @@ gk104_ram_ctor_data(struct gk104_ram *ram, u8 ramcfg, int i)
data = nvbios_timingEp(bios, cfg->bios.ramcfg_timing,
&ver, &hdr, &cnt, &len,
&cfg->bios);
- if (ret = -EINVAL, !data)
+ ret = -EINVAL;
+ if (!data)
goto done;
- if (ret = -ENOSYS, ver != 0x20 || hdr < 0x33)
+ ret = -ENOSYS;
+ if (ver != 0x20 || hdr < 0x33)
goto done;
}
list_add_tail(&cfg->head, &ram->cfg);
- if (ret = 0, i == 0)
+ ret = 0;
+ if (i == 0)
goto done;
d->rammap_11_0a_03fe |= p->rammap_11_0a_03fe != n->rammap_11_0a_03fe;
--
2.1.4
Powered by blists - more mailing lists