re: wireless: mwifiex: initial commit for Marvell mwifiex driver

re: wireless: mwifiex: initial commit for Marvell mwifiex driver

[Dan Carpenter]

Hello Bing Zhao,

The patch 5e6e3a92b9a4: "wireless: mwifiex: initial commit for
Marvell mwifiex driver" from Mar 21, 2011, leads to the following
static checker warning:

	drivers/net/wireless/marvell/mwifiex/sta_event.c:634 mwifiex_process_sta_event()
	warn: inconsistent indenting

drivers/net/wireless/marvell/mwifiex/sta_event.c
   608          case EVENT_PS_AWAKE:
   609                  mwifiex_dbg(adapter, EVENT, "info: EVENT: AWAKE\n");
   610                  if (!adapter->pps_uapsd_mode && priv->port_open &&
   611                      priv->media_connected && adapter->sleep_period.period) {
   612                                  adapter->pps_uapsd_mode = true;
   613                                  mwifiex_dbg(adapter, EVENT,
   614                                              "event: PPS/UAPSD mode activated\n");
   615                  }
   616                  adapter->tx_lock_flag = false;
   617                  if (adapter->pps_uapsd_mode && adapter->gen_null_pkt) {
   618                          if (mwifiex_check_last_packet_indication(priv)) {
   619                                  if (adapter->data_sent ||
   620                                      (adapter->if_ops.is_port_ready &&
   621                                       !adapter->if_ops.is_port_ready(priv))) {
   622                                          adapter->ps_state = PS_STATE_AWAKE;
   623                                          adapter->pm_wakeup_card_req = false;
   624                                          adapter->pm_wakeup_fw_try = false;
   625                                          del_timer(&adapter->wakeup_timer);
   626                                          break;
   627                                  }
   628                                  if (!mwifiex_send_null_packet
   629                                          (priv,
   630                                           MWIFIEX_TxPD_POWER_MGMT_NULL_PACKET |
   631                                           MWIFIEX_TxPD_POWER_MGMT_LAST_PACKET))
   632                                                  adapter->ps_state =
   633                                                          PS_STATE_SLEEP;
   634                                          return 0;


The PS_STATE_SLEEP line is indented one tab more than it should be.  It
looks like the code is buggy?  Possibly curly braces were intended:

					if (!mwifiex_send_null_packet(priv,
                                                    MWIFIEX_TxPD_POWER_MGMT_NULL_PACKET |
                                                    MWIFIEX_TxPD_POWER_MGMT_LAST_PACKET)) {
                                                adapter->ps_state = PS_STATE_SLEEP;
                                                return 0;
					}

We're right up against the 80 character limit so it's hard to be sure.

   635                          }
   636                  }
   637                  adapter->ps_state = PS_STATE_AWAKE;
   638                  adapter->pm_wakeup_card_req = false;
   639                  adapter->pm_wakeup_fw_try = false;
   640                  del_timer(&adapter->wakeup_timer);
   641  
   642                  break;
   643  
   644          case EVENT_DEEP_SLEEP_AWAKE:

regards,
dan carpenter

re: wireless: mwifiex: initial commit for Marvell mwifiex driver

[Dan Carpenter]

Hello Bing Zhao,

The patch 5e6e3a92b9a4: "wireless: mwifiex: initial commit for
Marvell mwifiex driver" from Mar 21, 2011, leads to the following
static checker warning:

	drivers/net/wireless/marvell/mwifiex/sta_cmd.c:133 mwifiex_cmd_802_11_snmp_mib()
	warn: potential memory corrupting cast 2 vs 1 bytes

drivers/net/wireless/marvell/mwifiex/sta_cmd.c
   112  static int mwifiex_cmd_802_11_snmp_mib(struct mwifiex_private *priv,
   113                                         struct host_cmd_ds_command *cmd,
   114                                         u16 cmd_action, u32 cmd_oid,
   115                                         u16 *ul_temp)
   116  {
   117          struct host_cmd_ds_802_11_snmp_mib *snmp_mib = &cmd->params.smib;
   118  
   119          mwifiex_dbg(priv->adapter, CMD,
   120                      "cmd: SNMP_CMD: cmd_oid = 0x%x\n", cmd_oid);
   121          cmd->command = cpu_to_le16(HostCmd_CMD_802_11_SNMP_MIB);
   122          cmd->size = cpu_to_le16(sizeof(struct host_cmd_ds_802_11_snmp_mib)
   123                                  - 1 + S_DS_GEN);
   124  
   125          snmp_mib->oid = cpu_to_le16((u16)cmd_oid);
   126          if (cmd_action == HostCmd_ACT_GEN_GET) {
   127                  snmp_mib->query_type = cpu_to_le16(HostCmd_ACT_GEN_GET);
   128                  snmp_mib->buf_size = cpu_to_le16(MAX_SNMP_BUF_SIZE);
   129                  le16_add_cpu(&cmd->size, MAX_SNMP_BUF_SIZE);
   130          } else if (cmd_action == HostCmd_ACT_GEN_SET) {
   131                  snmp_mib->query_type = cpu_to_le16(HostCmd_ACT_GEN_SET);
   132                  snmp_mib->buf_size = cpu_to_le16(sizeof(u16));
   133                  *((__le16 *) (snmp_mib->value)) = cpu_to_le16(*ul_temp);
                                      ^^^^^^^^^^^^^^^
We're writing two bytes here but it's only a 1 byte array.  It's a
__packed array as well so there isn't any padding.

   134                  le16_add_cpu(&cmd->size, sizeof(u16));
   135          }
   136  
   137          mwifiex_dbg(priv->adapter, CMD,
   138                      "cmd: SNMP_CMD: Action=0x%x, OID=0x%x,\t"
   139                      "OIDSize=0x%x, Value=0x%x\n",
   140                      cmd_action, cmd_oid, le16_to_cpu(snmp_mib->buf_size),
   141                      le16_to_cpu(*(__le16 *)snmp_mib->value));
   142          return 0;
   143  }



regards,
dan carpenter