bcma: fix null pointer in bcma_core_pci_irq_ctl
PATCH 1/1 bcma: fix null pointer in bcma_core_pci_irq_ctl
pc could be null if hosttype != BCMA_HOSTTYPE_PCI.
If we are on a device without a pci core this function is called with pc = null by b43 and brcmsmac. If the host type is PCI we have a pci core as well and pc can not be null.
--- drivers/bcma/driver_pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/bcma/driver_pci.c +3 -1
@@ -232,7 +232,7 @@ void __devinit bcma_core_pci_init(struct bcma_drv_pci *pc)int bcma_core_pci_irq_ctl (struct bcma_drv_pci *pc,struct bcma_device *core,bool enable) { -struct pci_dev *pdev = pc->core->bus->host_pci; +struct pci_dev *pdev;u32 coremask, tmp;int err =0 ;
@@ -243,6 +243,8 @@ int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core, goto out; } + pdev = pc->core->bus->host_pci; + err =pci_read_config_dword (pdev,BCMA_PCI_IRQMASK , &tmp);if (err) goto out;
Signature · 1 line
1.7.9.5
PATCH 1/1 bcma: fix null pointer in bcma_core_pci_irq_ctl
drivers/bcma/driver_pci.c int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core,
Quoted 24 lines · Hauke Mehrtens
pc could be null if hosttype !=
BCMA_HOSTTYPE_PCI.
If we are on a device without a pci core this function is called with pc = null by b43 and brcmsmac. If the host type is PCI we have a pci core as well and pc can not be null.Signed-off-by Hauke Mehrtens <[email protected]>--- drivers/bcma/driver_pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/bcma/driver_pci.c +1
-1@@ -232,7 +232,7 @@ void __devinit bcma_core_pci_init(struct bcma_drv_pci *pc)int bcma_core_pci_irq_ctl (struct bcma_drv_pci *pc,struct bcma_device *core,bool enable) { -struct pci_dev *pdev = pc->core->bus->host_pci; +struct pci_dev *pdev;u32 coremask, tmp;int err =0 ;@@ -243,6 +243,8 @@ int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core,
Could you change the if statement as well:
- if (core->bus->hosttype != BCMA_HOSTTYPE_PCI) {
+ if (!pc || core->bus->hosttype != BCMA_HOSTTYPE_PCI)
drivers/bcma/driver_pci.c int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core, · line 243
Quoted 1 line
drivers/bcma/driver_pci.c +0
-0goto out;
- }
drivers/bcma/driver_pci.c int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core, · line 250
Quoted 6 lines
drivers/bcma/driver_pci.c +2
-0+ pdev = pc->core->bus->host_pci; + err =pci_read_config_dword (pdev,BCMA_PCI_IRQMASK , &tmp);if (err) goto out;
Gr. AvS
PATCH 1/1 bcma: fix null pointer in bcma_core_pci_irq_ctl
Quoted 24 lines · Arend van Spriel
Quoted 23 lines · Hauke Mehrtens
pc could be null if hosttype !=
BCMA_HOSTTYPE_PCI.
If we are on a device without a pci core this function is called with pc = null by b43 and brcmsmac. If the host type is PCI we have a pci core as well and pc can not be null.Signed-off-by Hauke Mehrtens <[email protected]>--- drivers/bcma/driver_pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/bcma/driver_pci.c +1
-1@@ -232,7 +232,7 @@ void __devinit bcma_core_pci_init(struct bcma_drv_pci *pc)int bcma_core_pci_irq_ctl (struct bcma_drv_pci *pc,struct bcma_device *core,bool enable) { -struct pci_dev *pdev = pc->core->bus->host_pci; +struct pci_dev *pdev;u32 coremask, tmp;int err =0 ;
Quoted 4 lines
drivers/bcma/driver_pci.c int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core,
Quoted 1 line
drivers/bcma/driver_pci.c +0
-0@@ -243,6 +243,8 @@ int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core,Could you change the if statement as well:
- if (core->bus->hosttype!=BCMA_HOSTTYPE_PCI) {
+ if (!pc ||core->bus->hosttype!=BCMA_HOSTTYPE_PCI)
Yes that's good I will change that, just to be save.
Quoted 5 lines
drivers/bcma/driver_pci.c int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core, · line 246
Quoted 4 lines
drivers/bcma/driver_pci.c +0
-0/* This bcma device is not on a PCI host-bus. So the IRQs are * not routed through the PCI core. * So we must not enable routing through the PCI core. */ goto out;- }
Is removing the braces correct here? There is just one line of code so it will compile and the braces are not necessary, but I think it would looks somehow strange because there is a 3 line comment in addition to the one line of code in that if block.
@Joe Perches what do you think about this?
Quoted 5 lines
Quoted 5 lines
drivers/bcma/driver_pci.c +2
-0+ pdev = pc->core->bus->host_pci; + err =pci_read_config_dword (pdev,BCMA_PCI_IRQMASK , &tmp);if (err)goto out;
Hauke
PATCH 1/1 bcma: fix null pointer in bcma_core_pci_irq_ctl
Quoted 36 lines · Hauke Mehrtens
Quoted 24 lines · Arend van Spriel
Quoted 23 lines · Hauke Mehrtens
pc could be null if hosttype !=
BCMA_HOSTTYPE_PCI.
If we are on a device without a pci core this function is called with pc = null by b43 and brcmsmac. If the host type is PCI we have a pci core as well and pc can not be null.Signed-off-by Hauke Mehrtens <[email protected]>--- drivers/bcma/driver_pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)drivers/bcma/driver_pci.c +1
-1@@ -232,7 +232,7 @@ void __devinit bcma_core_pci_init(struct bcma_drv_pci *pc)int bcma_core_pci_irq_ctl (struct bcma_drv_pci *pc,struct bcma_device *core,bool enable) { -struct pci_dev *pdev = pc->core->bus->host_pci; +struct pci_dev *pdev;u32 coremask, tmp;int err =0 ;Quoted 4 lines
drivers/bcma/driver_pci.c int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core,
Quoted 1 line
drivers/bcma/driver_pci.c +0
-0@@ -243,6 +243,8 @@ int bcma_core_pci_irq_ctl(struct bcma_drv_pci *pc, struct bcma_device *core,Could you change the if statement as well:
- if (core->bus->hosttype!=BCMA_HOSTTYPE_PCI) {
+ if (!pc ||core->bus->hosttype!=BCMA_HOSTTYPE_PCI)Yes that's good I will change that, just to be save.
Quoted 4 lines
Quoted 4 lines
drivers/bcma/driver_pci.c +0
-0/* This bcma device is not on a PCI host-bus. So the IRQs are * not routed through the PCI core. * So we must not enable routing through the PCI core. */ goto out;
Quoted 8 lines
Quoted 1 line
- }
Is removing the braces correct here? There is just one line of code so it will compile and the braces are not necessary, but I think it would looks somehow strange because there is a 3 line comment in addition to the one line of code in that if block.
@Joe Perches what do you think about this?
I try not to tell others what they must do, I just try to suggest more standard ways to do things.
I think it's better to use braces when there's a multiline comment after a test with a single statement.
if (some_test()) {
/* a multiline
* comment
*/
single_statement(action);
}But I would tend to move that sort of comment above the fold and use something like:
/* If the bcma device is not on a PCI host-bus, * IRQs are not routed through the PCI core. */ if (!pc || core->bus->hosttype != BCMA_HOSTTYPE_PCI) goto out;
Feel free to do what you you think best.
cheers, Joe