Ticket #725 (closed defect: fixed)

Opened 6 years ago

Last modified 5 years ago

Last fix (Changeset 1657) by proski to ath/if_ath_pci.c does not compile

Reported by: szym@mit.edu Assigned to:
Priority: major Milestone: version 0.9.2
Component: madwifi: makefiles Version: trunk
Keywords: compilation warning Cc:
Patch is attached: 1 Pending:

Description

The fix was supposed to: Fix "empty body in an if-statement" warning

But now the compilation returns:

/root/more/madwifi-ng/ath/if_ath_pci.c: In function 'ath_pci_suspend': /root/more/madwifi-ng/ath/if_ath_pci.c:268: warning: ignoring return value of 'pci_set_power_state', declared with attribute warn_unused_result

Suggested patch: 

Index: ath/if_ath_pci.c
===================================================================
--- ath/if_ath_pci.c    (revision 1659)
+++ ath/if_ath_pci.c    (working copy)
@@ -265,7 +265,8 @@
        ath_suspend(dev);
        PCI_SAVE_STATE(pdev, ((struct ath_pci_softc *)dev->priv)->aps_pmstate);
        pci_disable_device(pdev);
-       pci_set_power_state(pdev, PCI_D3hot);
+       if(pci_set_power_state(pdev, PCI_D3hot))
+               return 1;

        return 0;
 }

Attachments

Change History

06/30/06 03:54:32 changed by mrenzmann

  • status changed from new to assigned.
  • owner set to mrenzmann.
  • milestone set to version 0.9.2.

Thanks, please sign off the patch so that it can be committed.

06/30/06 05:06:45 changed by szym@mit.edu

The patch removes the compilation warning for ath/if_ath_pci.c.

Signed-off-by: Szymon Chachulski <szym@mit.edu>

07/21/06 07:18:45 changed by proski

I think the patch is wrong. We are not supposed to return 1. Returning negated errno would be more logical. Although it's rare that the suspend function returns anything but zero, all the examples I could find in the kernel return -errno:

sc1200_suspend() in drivers/ide/pci/sc1200.c can return -ENOMEM. usb_hcd_pci_suspend() in drivers/usb/core/hcd-pci.c can return -EBUSY.

Returning 1 would be like "one device suspended" or something like that.

Besides, what kernel is that? Not checking the result of pci_set_power_state() is so common, I cannot imagine anyone enforcing this check without a good reason and a patch that takes care of such invocations at least in some parts of the kernel.

07/21/06 08:09:50 changed by szym@mit.edu

You are right. It's probably best to return the value returned by the call. 

Index: if_ath_pci.c
===================================================================
--- if_ath_pci.c	(revision 1687)
+++ if_ath_pci.c	(working copy)
@@ -265,9 +265,7 @@
 	ath_suspend(dev);
 	PCI_SAVE_STATE(pdev, ((struct ath_pci_softc *)dev->priv)->aps_pmstate);
 	pci_disable_device(pdev);
-	pci_set_power_state(pdev, PCI_D3hot);
-
-	return 0;
+	return pci_set_power_state(pdev, PCI_D3hot);
 }
 
 static int

Signed-off-by: Szymon Chachulski <szym@mit.edu>

The kernel I used was 2.6.11-1.1369_FC4-i686, which has the "must_check" for pci_set_power_state().

07/21/06 09:17:09 changed by mrenzmann

  • status changed from assigned to new.
  • owner deleted.

I'll remove assignment of this ticket to me, someone else please take over.

07/21/06 10:01:53 changed by proski

Fixed in r1688

07/21/06 10:14:37 changed by kelmo

  • status changed from new to closed.
  • resolution set to fixed.

Closing

Add/Change #725 (Last fix (Changeset 1657) by proski to ath/if_ath_pci.c does not compile)