Bug 188841 - Function typhoon_init_one() does not set error codes on failures
Summary: Function typhoon_init_one() does not set error codes on failures
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Network (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_network@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 11:02 UTC by bianpan
Modified: 2017-11-30 08:52 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description bianpan 2016-11-25 11:02:39 UTC
In function typhoon_init_one() defined in file drivers/net/ethernet/3com/typhoon.c, on failures, it sometimes returns negative error codes, sometimes returns 0 (indicates success by reviewing its contexts). I am not sure whether it is the author's intention or mistakes. I list 3 positions that seems anomalous. (1) when the call to is_valid_ether_addr() (at line 2401) fails, no error code is set; (2) when the call to typhoon_issue_command() (at line 2410) fails, no error code is set; (3) when the call to register_netdev() (at line 2452) fails, no error code is set. Codes related to these bugs are summarised as follows.

typhoon_init_one @@ drivers/net/ethernet/3com/typhoon.c
2261 static int
2262 typhoon_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
2263 {
         ...
2272     int err = 0;
         ...
2324     err = pci_request_regions(pdev, KBUILD_MODNAME);
2325     if(err < 0) {
2326         err_msg = "could not request regions";
2327         goto error_out_mwi;
2328     }
         // after the check of err at line 2325, the value of err must not be less than 0
         ...
2401     if(!is_valid_ether_addr(dev->dev_addr)) {
2402         err_msg = "Could not obtain valid ethernet address, aborting";
             // (1) insert "err = -EIO;" here?
2403         goto error_out_reset;
2404     }
2405 
2406     /* Read the Sleep Image version last, so the response is valid
2407      * later when we print out the version reported.
2408      */
2409     INIT_COMMAND_WITH_RESPONSE(&xp_cmd, TYPHOON_CMD_READ_VERSIONS);
2410     if(typhoon_issue_command(tp, 1, &xp_cmd, 3, xp_resp) < 0) {
2411         err_msg = "Could not get Sleep Image version";
             // (2) insert "err = -EIO;" here?
2412         goto error_out_reset;
2413     }
         ...
2452     if(register_netdev(dev) < 0) {
2453         err_msg = "unable to register netdev";
             // (3) insert "err = -EIO;" here?
2454         goto error_out_reset;
2455     }
         ...

2489     return 0;
2490 
2491 error_out_reset:
2492     typhoon_reset(ioaddr, NoWait);
2493 
2494 error_out_dma:
2495     pci_free_consistent(pdev, sizeof(struct typhoon_shared),
2496                 shared, shared_dma);
2497 error_out_remap:
2498     pci_iounmap(pdev, ioaddr);
2499 error_out_regions:
2500     pci_release_regions(pdev);
2501 error_out_mwi:
2502     pci_clear_mwi(pdev);
2503 error_out_disable:
2504     pci_disable_device(pdev);
2505 error_out_dev:
2506     free_netdev(dev);
2507 error_out:
2508     pr_err("%s: %s\n", pci_name(pdev), err_msg);
2509     return err;
2510 }

Thanks very much!
Comment 1 bianpan 2017-11-30 08:52:59 UTC
Fixed with the patch: https://patchwork.kernel.org/patch/10079273/.

Note You need to log in before you can comment on or make changes to this bug.