[bug report] ACPI, APEI, Add PCIe AER error information printing support

[bug report] ACPI, APEI, Add PCIe AER error information printing support

[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) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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

Re: [bug report] ACPI, APEI, Add PCIe AER error information printing support

[Bjorn Helgaas]

On Thu, Oct 13, 2016 at 09:58:47AM +0300, Dan Carpenter wrote:
> 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

Re: [bug report] ACPI, APEI, Add PCIe AER error information printing support

[Dan Carpenter]

On Thu, Oct 13, 2016 at 12:55:13PM -0500, Bjorn Helgaas wrote:
> On Thu, Oct 13, 2016 at 09:58:47AM +0300, Dan Carpenter wrote:
> > 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

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

regards,
dan carpenter

Re: [bug report] ACPI, APEI, Add PCIe AER error information printing support

[Huang, Ying]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> 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.

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

>    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