From 6f686d3d14621b90f3793b705bdf9fa624fd29ca Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Mon, 16 Jul 2007 21:25:01 -0400 Subject: [PATCH 01/11] kernel/auditfilter: kill bogus uninit'd-var compiler warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kill this warning... kernel/auditfilter.c: In function ‘audit_receive_filter’: kernel/auditfilter.c:1213: warning: ‘ndw’ may be used uninitialized in this function kernel/auditfilter.c:1213: warning: ‘ndp’ may be used uninitialized in this function ...with a simplification of the code. audit_put_nd() can accept NULL arguments, just like kfree(). It is cleaner to init two existing vars to NULL, remove the redundant test variable 'putnd_needed' branches, and call audit_put_nd() directly. As a desired side effect, the warning goes away. Signed-off-by: Jeff Garzik --- kernel/auditfilter.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index ce61f423542c..1bf093dcffe0 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -1210,8 +1210,8 @@ static inline int audit_add_rule(struct audit_entry *entry, struct audit_entry *e; struct audit_field *inode_f = entry->rule.inode_f; struct audit_watch *watch = entry->rule.watch; - struct nameidata *ndp, *ndw; - int h, err, putnd_needed = 0; + struct nameidata *ndp = NULL, *ndw = NULL; + int h, err; #ifdef CONFIG_AUDITSYSCALL int dont_count = 0; @@ -1239,7 +1239,6 @@ static inline int audit_add_rule(struct audit_entry *entry, err = audit_get_nd(watch->path, &ndp, &ndw); if (err) goto error; - putnd_needed = 1; } mutex_lock(&audit_filter_mutex); @@ -1269,14 +1268,11 @@ static inline int audit_add_rule(struct audit_entry *entry, #endif mutex_unlock(&audit_filter_mutex); - if (putnd_needed) - audit_put_nd(ndp, ndw); - + audit_put_nd(ndp, ndw); /* NULL args OK */ return 0; error: - if (putnd_needed) - audit_put_nd(ndp, ndw); + audit_put_nd(ndp, ndw); /* NULL args OK */ if (watch) audit_put_watch(watch); /* tmp watch, matches initial get */ return err; From f6c4286590e7cb13dd16cb2a6e4dc4a27ce6df1d Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 00:01:09 -0400 Subject: [PATCH 02/11] [netdrvr] natsemi: Fix device removal bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This episode illustrates how an overused warning can train people to ignore that warning, which winds up hiding bugs. The warning drivers/net/natsemi.c: In function ‘natsemi_remove1’: drivers/net/natsemi.c:3222: warning: ignoring return value of ‘device_create_file’, declared with attribute warn_unused_result is oft-ignored, even though at close inspection one notices this occurs in the /remove/ function, not normally where creation occurs. A quick s/create/remove/ and we are fixed, with the warning gone. Signed-off-by: Jeff Garzik --- drivers/net/natsemi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c index 3450051ae56b..6bb48ba80964 100644 --- a/drivers/net/natsemi.c +++ b/drivers/net/natsemi.c @@ -671,7 +671,7 @@ static ssize_t natsemi_show_##_name(struct device *dev, \ #define NATSEMI_CREATE_FILE(_dev, _name) \ device_create_file(&_dev->dev, &dev_attr_##_name) #define NATSEMI_REMOVE_FILE(_dev, _name) \ - device_create_file(&_dev->dev, &dev_attr_##_name) + device_remove_file(&_dev->dev, &dev_attr_##_name) NATSEMI_ATTR(dspcfg_workaround); From cad1b9da74f14c5f15b63ffc93c53debe09b3781 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 00:15:54 -0400 Subject: [PATCH 03/11] [netdrvr] eepro100, ne2k-pci: abort resume if pci_enable_device() fails Signed-off-by: Jeff Garzik --- drivers/net/eepro100.c | 7 ++++++- drivers/net/ne2k-pci.c | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/net/eepro100.c b/drivers/net/eepro100.c index 9afa47edfc58..3c54014acece 100644 --- a/drivers/net/eepro100.c +++ b/drivers/net/eepro100.c @@ -2292,10 +2292,15 @@ static int eepro100_resume(struct pci_dev *pdev) struct net_device *dev = pci_get_drvdata (pdev); struct speedo_private *sp = netdev_priv(dev); void __iomem *ioaddr = sp->regs; + int rc; pci_set_power_state(pdev, PCI_D0); pci_restore_state(pdev); - pci_enable_device(pdev); + + rc = pci_enable_device(pdev); + if (rc) + return rc; + pci_set_master(pdev); if (!netif_running(dev)) diff --git a/drivers/net/ne2k-pci.c b/drivers/net/ne2k-pci.c index 995c0a5d4066..cfdeaf7aa163 100644 --- a/drivers/net/ne2k-pci.c +++ b/drivers/net/ne2k-pci.c @@ -669,10 +669,15 @@ static int ne2k_pci_suspend (struct pci_dev *pdev, pm_message_t state) static int ne2k_pci_resume (struct pci_dev *pdev) { struct net_device *dev = pci_get_drvdata (pdev); + int rc; pci_set_power_state(pdev, 0); pci_restore_state(pdev); - pci_enable_device(pdev); + + rc = pci_enable_device(pdev); + if (rc) + return rc; + NS8390_init(dev, 1); netif_device_attach(dev); From ae97fec3701a559929c3529e35417fab133a4d39 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 01:08:29 -0400 Subject: [PATCH 04/11] drivers/usb/misc/auerswald: fix status check, remove redundant check 1) We should only set 'actual_length' output variable if usb length is known to be good. 2) No need to check actual_length for NULL. The only caller always passes non-NULL value. Signed-off-by: Jeff Garzik --- drivers/usb/misc/auerswald.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/misc/auerswald.c b/drivers/usb/misc/auerswald.c index 1fd5fc220cd7..3e22b2ff9e74 100644 --- a/drivers/usb/misc/auerswald.c +++ b/drivers/usb/misc/auerswald.c @@ -630,7 +630,7 @@ static int auerchain_start_wait_urb (pauerchain_t acp, struct urb *urb, int time } else status = urb->status; - if (actual_length) + if (status >= 0) *actual_length = urb->actual_length; return status; From 79c63e1976df035dee587c016d79cbccb130494a Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 01:32:29 -0400 Subject: [PATCH 05/11] drivers/net/wan/pc300_drv: fix bug caught by gcc warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The warning drivers/net/wan/pc300_drv.c: In function ‘cpc_open’: drivers/net/wan/pc300_drv.c:2942: warning: ‘br’ may be used uninitialized in this function was valid. Ensure 'br' is initialized in all cases. Signed-off-by: Jeff Garzik --- drivers/net/wan/pc300_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wan/pc300_drv.c b/drivers/net/wan/pc300_drv.c index ec1c556a47ca..5d8c78ee2cd9 100644 --- a/drivers/net/wan/pc300_drv.c +++ b/drivers/net/wan/pc300_drv.c @@ -2833,6 +2833,8 @@ static int clock_rate_calc(uclong rate, uclong clock, int *br_io) int br, tc; int br_pwr, error; + *br_io = 0; + if (rate == 0) return (0); From 0d480db85dea59e1393c3968fbdac0117431e797 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 01:35:08 -0400 Subject: [PATCH 06/11] drivers/telephony/ixj: cleanup and fix gcc warning 1) Fix gcc uninit'd var warnings by adding 'default' switch stmt labels in two cases. It was lightning-strikes unlikely that a problem would ever arise, but not impossible. 2) Tighten the scope of 'blankword' in two cases. Signed-off-by: Jeff Garzik --- drivers/telephony/ixj.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/telephony/ixj.c b/drivers/telephony/ixj.c index c7b0a357b04a..49cd9793404f 100644 --- a/drivers/telephony/ixj.c +++ b/drivers/telephony/ixj.c @@ -3453,7 +3453,6 @@ static void ixj_write_frame(IXJ *j) { int cnt, frame_count, dly; IXJ_WORD dat; - BYTES blankword; frame_count = 0; if(j->flags.cidplay) { @@ -3501,6 +3500,8 @@ static void ixj_write_frame(IXJ *j) } if (frame_count >= 1) { if (j->ver.low == 0x12 && j->play_mode && j->flags.play_first_frame) { + BYTES blankword; + switch (j->play_mode) { case PLAYBACK_MODE_ULAW: case PLAYBACK_MODE_ALAW: @@ -3508,6 +3509,7 @@ static void ixj_write_frame(IXJ *j) break; case PLAYBACK_MODE_8LINEAR: case PLAYBACK_MODE_16LINEAR: + default: blankword.low = blankword.high = 0x00; break; case PLAYBACK_MODE_8LINEAR_WSS: @@ -3531,6 +3533,8 @@ static void ixj_write_frame(IXJ *j) j->flags.play_first_frame = 0; } else if (j->play_codec == G723_63 && j->flags.play_first_frame) { for (cnt = 0; cnt < 24; cnt++) { + BYTES blankword; + if(cnt == 12) { blankword.low = 0x02; blankword.high = 0x00; @@ -4868,6 +4872,7 @@ static char daa_CR_read(IXJ *j, int cr) bytes.high = 0xB0 + cr; break; case SOP_PU_PULSEDIALING: + default: bytes.high = 0xF0 + cr; break; } From 2ab934b8afa89b9b3e71b7fb66470a19772f5012 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 01:49:56 -0400 Subject: [PATCH 07/11] drivers/mtd/ubi/eba: minor cleanup: tighten scope of a local var Signed-off-by: Jeff Garzik --- drivers/mtd/ubi/eba.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c index 74002945b71b..4dc10c8ab964 100644 --- a/drivers/mtd/ubi/eba.c +++ b/drivers/mtd/ubi/eba.c @@ -368,7 +368,7 @@ int ubi_eba_read_leb(struct ubi_device *ubi, int vol_id, int lnum, void *buf, int err, pnum, scrub = 0, idx = vol_id2idx(ubi, vol_id); struct ubi_vid_hdr *vid_hdr; struct ubi_volume *vol = ubi->volumes[idx]; - uint32_t crc, crc1; + uint32_t crc; err = leb_read_lock(ubi, vol_id, lnum); if (err) @@ -451,7 +451,7 @@ retry: } if (check) { - crc1 = crc32(UBI_CRC32_INIT, buf, len); + uint32_t crc1 = crc32(UBI_CRC32_INIT, buf, len); if (crc1 != crc) { ubi_warn("CRC error: calculated %#08x, must be %#08x", crc1, crc); From e5fb4f42268654ca41ab50b1406fb7da97559db5 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 01:56:32 -0400 Subject: [PATCH 08/11] drivers/net/wan/sbni: kill uninit'd var warning It's actually convenient in the code to initialize this and a sister variable to zero. Signed-off-by: Jeff Garzik --- drivers/net/wan/sbni.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/wan/sbni.c b/drivers/net/wan/sbni.c index 35eded7ffb2d..1cc18e787a65 100644 --- a/drivers/net/wan/sbni.c +++ b/drivers/net/wan/sbni.c @@ -595,8 +595,8 @@ recv_frame( struct net_device *dev ) u32 crc = CRC32_INITIAL; - unsigned framelen, frameno, ack; - unsigned is_first, frame_ok; + unsigned framelen = 0, frameno, ack; + unsigned is_first, frame_ok = 0; if( check_fhdr( ioaddr, &framelen, &frameno, &ack, &is_first, &crc ) ) { frame_ok = framelen > 4 @@ -604,8 +604,7 @@ recv_frame( struct net_device *dev ) : skip_tail( ioaddr, framelen, crc ); if( frame_ok ) interpret_ack( dev, ack ); - } else - frame_ok = 0; + } outb( inb( ioaddr + CSR0 ) ^ CT_ZER, ioaddr + CSR0 ); if( frame_ok ) { From 9db48926208562df3c778682e064990170ab8971 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 02:03:49 -0400 Subject: [PATCH 09/11] drivers/infiniband/hw/mthca/mthca_qp: kill uninit'd var warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drivers/infiniband/hw/mthca/mthca_qp.c: In function ‘mthca_tavor_post_send’: drivers/infiniband/hw/mthca/mthca_qp.c:1594: warning: ‘f0’ may be used uninitialized in this function drivers/infiniband/hw/mthca/mthca_qp.c: In function ‘mthca_arbel_post_send’: drivers/infiniband/hw/mthca/mthca_qp.c:1949: warning: ‘f0’ may be used uninitialized in this function Initializing 'f0' is not strictly necessary in either case, AFAICS. I was considering use of uninitialized_var(), but looking at the complex flow of control in each function, I feel it is wiser and safer to simply zero the var and be certain of ourselves. Signed-off-by: Jeff Garzik --- drivers/infiniband/hw/mthca/mthca_qp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/hw/mthca/mthca_qp.c b/drivers/infiniband/hw/mthca/mthca_qp.c index eef415b12b2e..11f1d99db40b 100644 --- a/drivers/infiniband/hw/mthca/mthca_qp.c +++ b/drivers/infiniband/hw/mthca/mthca_qp.c @@ -1591,7 +1591,7 @@ int mthca_tavor_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr, int i; int size; int size0 = 0; - u32 f0; + u32 f0 = 0; int ind; u8 op0 = 0; @@ -1946,7 +1946,7 @@ int mthca_arbel_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr, int i; int size; int size0 = 0; - u32 f0; + u32 f0 = 0; int ind; u8 op0 = 0; From ea8b4db97aa41a66c05daa4055a1974692ccd52d Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 02:21:50 -0400 Subject: [PATCH 10/11] [libata] sata_mv: use pci_try_set_mwi() Because sometimes in life, it's ok to fail. Signed-off-by: Jeff Garzik --- drivers/ata/sata_mv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 5d576435fccc..fb8a749423ca 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -2666,7 +2666,7 @@ static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) mv_print_info(host); pci_set_master(pdev); - pci_set_mwi(pdev); + pci_try_set_mwi(pdev); return ata_host_activate(host, pdev->irq, mv_interrupt, IRQF_SHARED, IS_GEN_I(hpriv) ? &mv5_sht : &mv6_sht); } From b1734d2388cc45ecdec58615e35955d0d402f938 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 17 Jul 2007 02:32:21 -0400 Subject: [PATCH 11/11] drivers/atm/ambassador: kill uninit'd var warning, and fix bug An uninitialized variable warning illuminated an area where indeed the variable was being used without initialization. Unfortunately, after verifying all such paths were fixed, the warning still appears. So we follow the initialization practice of other variables in this function. Signed-off-by: Jeff Garzik --- drivers/atm/ambassador.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c index 59651abfa4f8..b34b3829f6a9 100644 --- a/drivers/atm/ambassador.c +++ b/drivers/atm/ambassador.c @@ -1040,7 +1040,7 @@ static int amb_open (struct atm_vcc * atm_vcc) struct atm_qos * qos; struct atm_trafprm * txtp; struct atm_trafprm * rxtp; - u16 tx_rate_bits; + u16 tx_rate_bits = -1; // hush gcc u16 tx_vc_bits = -1; // hush gcc u16 tx_frame_bits = -1; // hush gcc @@ -1096,6 +1096,8 @@ static int amb_open (struct atm_vcc * atm_vcc) r = round_up; } error = make_rate (pcr, r, &tx_rate_bits, NULL); + if (error) + return error; tx_vc_bits = TX_UBR_CAPPED; tx_frame_bits = TX_FRAME_CAPPED; }