Project

General

Profile

Actions

Bug #15489

closed

cfgadm_plugins: the comparison will always evaluate as 'true'

Added by Toomas Soome 3 months ago. Updated 2 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

Build errors with gcc 12:

    ../common/shp.c: In function 'find_physical_slot_names':
    ../common/shp.c:1166:35: error: the comparison will always evaluate as 'true' for the address of 'slotnames' will never be NULL [-Werror=address]
     1166 |         if (slotarg->slotnames[0] != NULL)
          |                                   ^~
    ../common/shp.c:274:17: note: 'slotnames' declared here
      274 |         char    slotnames[MAXDEVS][MAXNAMELEN];
          |                 ^~~~~~~~~
    cc1: all warnings being treated as errors

    ../common/cfga.c: In function 'find_physical_slot_names':
    ../common/cfga.c:1296:35: error: the comparison will always evaluate as 'true' for the address of 'slotnames' will never be NULL [-Werror=address]
     1296 |         if (slotarg->slotnames[0] != NULL)
          |                                   ^~
    ../common/cfga.c:261:17: note: 'slotnames' declared here
      261 |         char    slotnames[MAXDEVS][MAXNAMELEN];
          |                 ^~~~~~~~~
    cc1: all warnings being treated as errors

Indeed, the arrays are allocated and therefore never NULL. Which always leads us to the question if the check is needed or if the condition statement is wrong.

Testing done: build, install, boot, cfgadm command appears to work as expected.
wsdiff does show differences in constants, but also the added call to memset() did cause additional noise.

Actions #1

Updated by Electric Monk 3 months ago

  • Gerrit CR set to 2720
Actions #2

Updated by Toomas Soome 2 months ago

  • Description updated (diff)
Actions #3

Updated by Toomas Soome 2 months ago

  • Description updated (diff)
  • Status changed from In Progress to Pending RTI
Actions #4

Updated by Robert Mustacchi 2 months ago

I went through and did a little bit of testing on a commodity server that had NVMe hotplug support. In particular I used cfgadm to manually unconfigure and configure the end point. This leveraged the shp plugin that was changed here.

Actions #5

Updated by Electric Monk 2 months ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 90 to 100

git commit 014ec826f15c2551c9f84a5fe6f816ffb5bee7da

commit  014ec826f15c2551c9f84a5fe6f816ffb5bee7da
Author: Toomas Soome <tsoome@me.com>
Date:   2023-03-31T16:41:14.000Z

    15489 cfgadm_plugins: the comparison will always evaluate as 'true'
    Reviewed by: Dan Cross <cross@oxidecomputer.com>
    Reviewed by: Gergő Mihály Doma <domag02@gmail.com>
    Approved by: Robert Mustacchi <rm@fingolfin.org>

Actions

Also available in: Atom PDF