Linux wireless drivers development
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Ajay Singh <ajay.kathat@microchip.com>
Cc: linux-wireless@vger.kernel.org, devel@driverdev.osuosl.org,
	venkateswara.kaja@microchip.com, gregkh@linuxfoundation.org,
	ganesh.krishna@microchip.com, adham.abozaeid@microchip.com,
	aditya.shankar@microchip.com
Subject: Re: [PATCH 29/29] staging: wilc1000: return exact error of register_netdev() from wilc_netdev_init()
Date: Wed, 19 Sep 2018 12:41:32 +0300	[thread overview]
Message-ID: <20180919094132.3lfouwwzc7z3thf4@mwanda> (raw)
In-Reply-To: <1537337119-14952-30-git-send-email-ajay.kathat@microchip.com>

I was waiting for you to send this like a spider waits for flies.  You
fell directly into my trap.  Mwuahahahahaha.

drivers/staging/wilc1000/linux_wlan.c
  1056  int wilc_netdev_init(struct wilc **wilc, struct device *dev, int io_type,
  1057                       const struct wilc_hif_func *ops)
  1058  {
  1059          int i, ret = -ENOMEM;
  1060          struct wilc_vif *vif;
  1061          struct net_device *ndev;
  1062          struct wilc *wl;
  1063  
  1064          wl = kzalloc(sizeof(*wl), GFP_KERNEL);
  1065          if (!wl)
  1066                  return ret;
                               ^^^
It's cleaner to return -ENOMEM so that we don't have to glance up to the
declaration block.  This is especially true when "ret" is zero, btw,
because that can indicate a reversed test.

		if (!ret)
			return ret;

In this theoretically example it was supposed to be:

		if (ret)
			return ret;

Normally, reversed conditions are caught in testing, but for the kernel,
no one has the hardware to test everything so we do get reversed
conditions from time to time.

  1067  
  1068          if (wilc_wlan_cfg_init(wl))
  1069                  goto free_wl;
  1070  
  1071          *wilc = wl;
  1072          wl->io_type = io_type;
  1073          wl->hif_func = ops;
  1074          wl->enable_ps = true;
  1075          wl->chip_ps_state = CHIP_WAKEDUP;
  1076          INIT_LIST_HEAD(&wl->txq_head.list);
  1077          INIT_LIST_HEAD(&wl->rxq_head.list);
  1078  
  1079          wl->hif_workqueue = create_singlethread_workqueue("WILC_wq");
  1080          if (!wl->hif_workqueue)
  1081                  goto free_cfg;
  1082  
  1083          register_inetaddr_notifier(&g_dev_notifier);
  1084  
  1085          for (i = 0; i < NUM_CONCURRENT_IFC; i++) {
  1086                  struct wireless_dev *wdev;
  1087  
  1088                  ndev = alloc_etherdev(sizeof(struct wilc_vif));
  1089                  if (!ndev)
  1090                          goto free_ndev;
                                ^^^^^^^^^^^^^^^
ret is zero on the second iteration through the loop.

  1091  
  1092                  vif = netdev_priv(ndev);
  1093                  memset(vif, 0, sizeof(struct wilc_vif));
  1094  
  1095                  if (i == 0) {
  1096                          strcpy(ndev->name, "wlan%d");
  1097                          vif->ifc_id = 1;
  1098                  } else {
  1099                          strcpy(ndev->name, "p2p%d");
  1100                          vif->ifc_id = 0;
  1101                  }
  1102                  vif->wilc = *wilc;
  1103                  vif->ndev = ndev;
  1104                  wl->vif[i] = vif;
  1105                  wl->vif_num = i;
  1106                  vif->idx = wl->vif_num;
  1107  
  1108                  ndev->netdev_ops = &wilc_netdev_ops;
  1109  
  1110                  wdev = wilc_create_wiphy(ndev, dev);
  1111                  if (!wdev) {
  1112                          netdev_err(ndev, "Can't register WILC Wiphy\n");
  1113                          goto free_ndev;
                                ^^^^^^^^^^^^^^^
Here too.

  1114                  }
  1115  
  1116                  SET_NETDEV_DEV(ndev, dev);
  1117  
  1118                  vif->ndev->ieee80211_ptr = wdev;
  1119                  vif->ndev->ml_priv = vif;
  1120                  wdev->netdev = vif->ndev;
  1121                  vif->netstats.rx_packets = 0;
  1122                  vif->netstats.tx_packets = 0;
  1123                  vif->netstats.rx_bytes = 0;
  1124                  vif->netstats.tx_bytes = 0;
  1125  
  1126                  ret = register_netdev(ndev);
  1127                  if (ret)
  1128                          goto free_ndev;
                        ^^^^^^^^
ret is cleared on the first iteration through the loop.

  1129  
  1130                  vif->iftype = STATION_MODE;
  1131                  vif->mac_opened = 0;
  1132          }
  1133  
  1134          return 0;

regards,
dan carpenter

  reply	other threads:[~2018-09-19 15:18 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19  6:04 [PATCH 00/29] staging: wilc1000: avoid static variables and cleanup changes Ajay Singh
2018-09-19  6:04 ` [PATCH 01/29] staging: wilc1000: change return type to 'void' for wilc_frame_register() Ajay Singh
2018-09-19  6:04 ` [PATCH 02/29] staging: wilc1000: change return type to 'void' for wilc_wlan_set_bssid() Ajay Singh
2018-09-19  6:04 ` [PATCH 03/29] staging: wilc1000: change return type to 'void' for lock init & deinit functions Ajay Singh
2018-09-19  6:04 ` [PATCH 04/29] staging: wilc1000: change return type to 'void' for wilc_deinit_host_int() Ajay Singh
2018-09-19  6:04 ` [PATCH 05/29] staging: wilc1000: change return type to 'void' for wilc_wfi_deinit_mon_interface() Ajay Singh
2018-09-19  6:04 ` [PATCH 06/29] staging: wilc1000: use 'void' return type for host_int_get_assoc_res_info() Ajay Singh
2018-09-19  6:04 ` [PATCH 07/29] staging: wilc1000: use 'void' return for wilc_wlan_txq_add_to_head() Ajay Singh
2018-09-19  6:04 ` [PATCH 08/29] staging: wilc1000: change return type to 'void' tcp ack filter functions Ajay Singh
2018-09-19  6:04 ` [PATCH 09/29] staging: wilc1000: use 'void' return for wilc_wlan_txq_filter_dup_tcp_ack() Ajay Singh
2018-09-19  6:05 ` [PATCH 10/29] staging: wilc1000: change return type to 'void' for wilc_wlan_cfg_indicate_rx() Ajay Singh
2018-09-19  6:05 ` [PATCH 11/29] staging: wilc1000: refactor wilc_wlan_parse_info_frame() function Ajay Singh
2018-09-19  6:05 ` [PATCH 12/29] staging: wilc1000: set default value of cfg response type in wilc_wlan_cfg_indicate_rx() Ajay Singh
2018-09-19  6:05 ` [PATCH 13/29] staging: wilc1000: changes 'val' type to u8 in wilc_cfg_byte struct Ajay Singh
2018-09-19  6:05 ` [PATCH 14/29] staging: wilc1000: remove unused wid type values Ajay Singh
2018-09-19  6:05 ` [PATCH 15/29] staging: wilc1000: remove unused wid from cfg struct Ajay Singh
2018-09-19  6:05 ` [PATCH 16/29] staging: wilc1000: refactor code to remove 'mac_status' from 'wilc_mac_cfg' struct Ajay Singh
2018-09-19  6:05 ` [PATCH 17/29] staging: wilc1000: refactor code to avoid static variables for config parameters Ajay Singh
2018-09-19  6:05 ` [PATCH 18/29] staging: wilc1000: rename 'wilc_mac_cfg' struct to 'wilc_cfg_str_vals' Ajay Singh
2018-09-19  6:05 ` [PATCH 19/29] staging: wilc1000: avoid the use of 'hif_driver_comp' completion variable Ajay Singh
2018-09-19  6:05 ` [PATCH 20/29] staging: wilc1000: remove use of unnecessary 'wilc_connected_ssid' variable Ajay Singh
2018-09-19  6:05 ` [PATCH 21/29] staging: wilc1000: avoid use of 'g_sdio' static variable Ajay Singh
2018-09-19  6:05 ` [PATCH 22/29] staging: wilc1000: avoid use of 'g_spi' " Ajay Singh
2018-09-19  6:05 ` [PATCH 23/29] staging: wilc1000: remove unnecessary memset in sdio_init() & wilc_spi_init() Ajay Singh
2018-09-19  6:05 ` [PATCH 24/29] staging: wilc1000: remove p2p related static variables to wilc_vif struct Ajay Singh
2018-09-19  6:05 ` [PATCH 25/29] staging: wilc1000: remove wilc_debugfs.c file as its not used Ajay Singh
2018-09-19  6:05 ` [PATCH 26/29] staging: wilc1000: remove unnecessary option used with ccflags-y in Makefile Ajay Singh
2018-09-19  6:05 ` [PATCH 27/29] staging: wilc1000: use usleep_range() in place of udelay() Ajay Singh
2018-09-19  6:05 ` [PATCH 28/29] staging: wilc1000: avoid spaces preferred around checkpatch issue Ajay Singh
2018-09-19  6:05 ` [PATCH 29/29] staging: wilc1000: return exact error of register_netdev() from wilc_netdev_init() Ajay Singh
2018-09-19  9:41   ` Dan Carpenter [this message]
2018-09-19 11:04     ` Ajay Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180919094132.3lfouwwzc7z3thf4@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=adham.abozaeid@microchip.com \
    --cc=aditya.shankar@microchip.com \
    --cc=ajay.kathat@microchip.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=ganesh.krishna@microchip.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=venkateswara.kaja@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox