Skip to contentSkip to conversation

drm/nouveau/bios: set info only when the return is not 0

PATCH In review Tom Rix 2 participants 4 messages Jul 2, 2022 – Jul 15, 2022

Subsystems (3)
lore
Share & tools
lore

Tom Rix

author
#0 lore raw

PATCH 1/1 drm/nouveau/bios: set info only when the return is not 0

clang static analysis reports
drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c:68:17: warning: The right operand of '*' is a garbage value [core.UndefinedBinaryOperatorResult]

        switch (!!data * *ver) {
                       ^ ~~~~

A switch statement with only a default should be reduced to an if.

If nvbios_pmuEp() returns 0, via the data variable, the output info parameter is not used. So set info only when data is not 0.

The struct nvbios_pmuE only has the type and data elements. Since both of these are explicitly set, memset is not needed. So remove it.

---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c +1 -4
index b4a308f3cf7b..49e2664a734c 100644
@@ -64,12 +64,9 @@ nvbios_pmuEp(struct nvkm_bios *bios, int idx, u8 *ver, u8 *hdr,
 	     struct nvbios_pmuE *info)
 {
 	u32 data = nvbios_pmuEe(bios, idx, ver, hdr);
-	memset(info, 0x00, sizeof(*info));
-	switch (!!data * *ver) {
-	default:
+	if (data) {
 		info->type = nvbios_rd08(bios, data + 0x00);
 		info->data = nvbios_rd32(bios, data + 0x02);
-		break;
 	}
 	return data;
 }
Signature · 1 line
2.27.0
maintainer Reviewed-by on v1 1/1
#1 lore raw

Currently on flakey internet in the back of a car, so I probably won't be able to push this until I get back tonight or early tomorrow

Quoted 40 lines · Tom Rix

clang static analysis reports drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c:68:17: warning: The right operand of '*' is a garbage value [core.UndefinedBinaryOperatorResult] switch (!!data * *ver) {
                       ^ ~~~~
A switch statement with only a default should be reduced to an if.

If nvbios_pmuEp() returns 0, via the data variable, the output info parameter
is not used.  So set info only when data is not 0.

The struct nvbios_pmuE only has the type and data elements.  Since both of these
are explicitly set, memset is not needed.  So remove it.

---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c +1 -4
index b4a308f3cf7b..49e2664a734c 100644
@@ -64,12 +64,9 @@ nvbios_pmuEp(struct nvkm_bios *bios, int idx, u8 *ver, u8
 *hdr,
              struct nvbios_pmuE *info)
  {
         u32 data = nvbios_pmuEe(bios, idx, ver, hdr);
-       memset(info, 0x00, sizeof(*info));
-       switch (!!data * *ver) {
-       default:
+       if (data) {
                 info->type = nvbios_rd08(bios, data + 0x00);
                 info->data = nvbios_rd32(bios, data + 0x02);
-               break;
         }
         return data;

 }

Signature · 3 lines
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat
maintainer
#2 lore raw
Quoted 5 lines · Lyude Paul

Currently on flakey internet in the back of a car, so I probably won't be able
to push this until I get back tonight or early tomorrow

Whoops! Slipped my mind when I got back, but I just remembered it now so I will go ahead and push :). apologies for the delay!

Quoted 44 lines · Tom Rix
Quoted 41 lines

clang static analysis reports drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c:68:17: warning: The right operand of '*' is a garbage value [core.UndefinedBinaryOperatorResult] switch (!!data * *ver) {
                       ^ ~~~~
A switch statement with only a default should be reduced to an if.

If nvbios_pmuEp() returns 0, via the data variable, the output info parameter
is not used.  So set info only when data is not 0.

The struct nvbios_pmuE only has the type and data elements.  Since both of these
are explicitly set, memset is not needed.  So remove it.

---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c | 5 +----

 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c +0 -0
index b4a308f3cf7b..49e2664a734c 100644
@@ -64,12 +64,9 @@ nvbios_pmuEp(struct nvkm_bios *bios, int idx, u8 *ver,

u8
*hdr,
             struct nvbios_pmuE *info)
 {
        u32 data = nvbios_pmuEe(bios, idx, ver, hdr); -       memset(info, 0x00, sizeof(*info));
-       switch (!!data * *ver) {
-       default:
+       if (data) {
                info->type = nvbios_rd08(bios, data + 0x00); info->data = nvbios_rd32(bios, data + 0x02); -               break;
        }
        return data;
 }

Signature · 3 lines
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat
author
#3 lore raw
Quoted 8 lines · Lyude Paul
Quoted 5 lines · Lyude Paul

Currently on flakey internet in the back of a car, so I probably won't be able
to push this until I get back tonight or early tomorrow

Whoops! Slipped my mind when I got back, but I just remembered it now so I will go ahead and push :). apologies for the delay!

Thanks!

T

Quoted 43 lines
Quoted 43 lines · Tom Rix
Quoted 41 lines

clang static analysis reports drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c:68:17: warning: The right operand of '*' is a garbage value [core.UndefinedBinaryOperatorResult]
        switch (!!data * *ver) {
                       ^ ~~~~
A switch statement with only a default should be reduced to an if.

If nvbios_pmuEp() returns 0, via the data variable, the output info parameter
is not used.  So set info only when data is not 0.

The struct nvbios_pmuE only has the type and data elements.  Since both of these
are explicitly set, memset is not needed.  So remove it.

---
  drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c | 5 +----
  1 file changed, 1 insertion(+), 4 deletions(-)
drivers/gpu/drm/nouveau/nvkm/subdev/bios/pmu.c +0 -0
index b4a308f3cf7b..49e2664a734c 100644
@@ -64,12 +64,9 @@ nvbios_pmuEp(struct nvkm_bios *bios, int idx, u8 *ver,

u8
*hdr,
             struct nvbios_pmuE *info)
 {
        u32 data = nvbios_pmuEe(bios, idx, ver, hdr);
-       memset(info, 0x00, sizeof(*info));
-       switch (!!data * *ver) {
-       default:
+       if (data) {
                info->type = nvbios_rd08(bios, data + 0x00); info->data = nvbios_rd32(bios, data + 0x02);
-               break;
        }
        return data;
 }