Skip to contentSkip to conversation

ACPI, APEI, Add PCIe AER error information printing support

bug report Dan Carpenter 3 participants 4 messages Oct 13, 2016 – Oct 14, 2016 PCI

lore
Share & tools
lore
#0 lore raw

Hello Huang Ying,

The patch c413d7682020: "ACPI, APEI, Add PCIe AER error information printing support" from Feb 21, 2011, leads to the following static checker warning:

drivers/pci/pcie/aer/aerdrv_errprint.c:229 cper_print_aer()
warn: bool comparison is always 'false'

drivers/pci/pcie/aer/aerdrv_errprint.c
222 void cper_print_aer(struct pci_dev *dev, int aer_severity,
223 struct aer_capability_regs *aer)
224 {

   225          int layer, agent, status_strs_size, tlp_header_valid = 0;
   226          u32 status, mask;
   227          const char **status_strs;
   228  
   229          if (aer_severity == AER_CORRECTABLE) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

In the current code, aer_severity can only be AER_NONFATAL or AER_FATAL so this condition can never be true. This seems like it a confusing thing.

   230                  status = aer->cor_status;
   231                  mask = aer->cor_mask;
   232                  status_strs = aer_correctable_error_string;
   233                  status_strs_size = ARRAY_SIZE(aer_correctable_error_string);
   234          } else {
   235                  status = aer->uncor_status;
   236                  mask = aer->uncor_mask;
   237                  status_strs = aer_uncorrectable_error_string;
   238                  status_strs_size = ARRAY_SIZE(aer_uncorrectable_error_string);
   239                  tlp_header_valid = status & AER_LOG_TLP_MASKS;

240 }
241
242 layer = AER_GET_LAYER_ERROR(aer_severity, status);
243 agent = AER_GET_AGENT(aer_severity, status);
244

regards,
dan carpenter

#1 lore raw
Quoted 8 lines · Dan Carpenter

Hello Huang Ying,

The patch c413d7682020: "ACPI, APEI, Add PCIe AER error information printing support" from Feb 21, 2011, leads to the following static checker warning:

drivers/pci/pcie/aer/aerdrv_errprint.c:229 cper_print_aer()
warn: bool comparison is always 'false'

Hi Dan,

Thanks a lot for the report. Do you have any plans for integrating smatch into the kernel build process like sparse is? I think I did figure out how to run smatch myself, but it wasn't quite as easy as "apt-get install sparse; make C=2".

Bjorn

#2 lore raw
Quoted 10 lines · Bjorn Helgaas
Quoted 8 lines · Dan Carpenter

Hello Huang Ying,

The patch c413d7682020: "ACPI, APEI, Add PCIe AER error information printing support" from Feb 21, 2011, leads to the following static checker warning:

drivers/pci/pcie/aer/aerdrv_errprint.c:229 cper_print_aer()
warn: bool comparison is always 'false'

Quoted 8 lines

Hi Dan,

Thanks a lot for the report. Do you have any plans for integrating smatch into the kernel build process like sparse is? I think I did figure out how to run smatch myself, but it wasn't quite as easy as "apt-get install sparse; make C=2".

Bjorn

I don't know anything about packaging... :/

regards,
dan carpenter

#3 lore raw
In reply to Dan Carpenter
Quoted 19 lines · Dan Carpenter

Hello Huang Ying,

The patch c413d7682020: "ACPI, APEI, Add PCIe AER error information printing support" from Feb 21, 2011, leads to the following static checker warning:

drivers/pci/pcie/aer/aerdrv_errprint.c:229 cper_print_aer()
warn: bool comparison is always 'false'

drivers/pci/pcie/aer/aerdrv_errprint.c
222 void cper_print_aer(struct pci_dev *dev, int aer_severity,
223 struct aer_capability_regs *aer)

   224  {
   225          int layer, agent, status_strs_size, tlp_header_valid = 0;
   226          u32 status, mask;
   227          const char **status_strs;
   228  
   229          if (aer_severity == AER_CORRECTABLE) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 3 lines

In the current code, aer_severity can only be AER_NONFATAL or AER_FATAL so this condition can never be true. This seems like it a confusing thing.

For now, there is no AER_CORRECTABLE usage for cper_print_aer(), but I think we will need it sometime.

But your tool is very smart! :)

Best Regards,
Huang, Ying

Quoted 18 lines
   230                  status = aer->cor_status;
   231                  mask = aer->cor_mask;
   232                  status_strs = aer_correctable_error_string;
   233                  status_strs_size = ARRAY_SIZE(aer_correctable_error_string);
   234          } else {
   235                  status = aer->uncor_status;
   236                  mask = aer->uncor_mask;
   237                  status_strs = aer_uncorrectable_error_string;
   238                  status_strs_size = ARRAY_SIZE(aer_uncorrectable_error_string);
   239                  tlp_header_valid = status & AER_LOG_TLP_MASKS;
   240          }
   241  
   242          layer = AER_GET_LAYER_ERROR(aer_severity, status);
   243          agent = AER_GET_AGENT(aer_severity, status);
   244  

regards,
dan carpenter