From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752344AbcESI1X (ORCPT <rfc822;w@1wt.eu>);
	Thu, 19 May 2016 04:27:23 -0400
Received: from mga09.intel.com ([134.134.136.24]:29024 "EHLO mga09.intel.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753711AbcESI1T (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Thu, 19 May 2016 04:27:19 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.26,333,1459839600"; 
   d="scan'208";a="106720353"
From: "Coelho, Luciano" <luciano.coelho@intel.com>
To: "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "xypron.glpk@gmx.de" <xypron.glpk@gmx.de>,
        "Berg, Johannes" <johannes.berg@intel.com>,
        "Grumbach, Emmanuel" <emmanuel.grumbach@intel.com>
CC: linuxwifi <linuxwifi@intel.com>, "eyal@wizery.com" <eyal@wizery.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "Greenman, Gregory" <gregory.greenman@intel.com>,
        "Bondar, Alexander" <alexander.bondar@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] iwlwifi: rs: remove superfluous check
Thread-Topic: [PATCH 1/1] iwlwifi: rs: remove superfluous check
Thread-Index: AQHRsJRQvx68KZxUZUqmaBgoAaRQIZ+/3pCA
Date: Thu, 19 May 2016 08:27:13 +0000
Message-ID: <1463646433.29999.50.camel@intel.com>
References: <1463527868-12226-1-git-send-email-xypron.glpk@gmx.de>
In-Reply-To: <1463527868-12226-1-git-send-email-xypron.glpk@gmx.de>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.252.29.4]
Content-Type: text/plain; charset="utf-8"
Content-ID: <563BCFB089518847B5367C0C20B37A64@intel.com>
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u4J8RUYb000583

On Wed, 2016-05-18 at 01:31 +0200, Heinrich Schuchardt wrote:
> If we dereference a variable anyway in other parts of the code,
> there is no need to check against NULL in a single place.

NACK.  This is not true.

If lq_sta is NULL, it means that mvm_sta is also NULL.  Then we call
the rate_control_send with mvm_sta == NULL:

	if (rate_control_send_low(sta, mvm_sta, txrc))
		return;

The rate_control_send_low() function looks like this:


bool rate_control_send_low(struct ieee80211_sta *pubsta,
			   void *priv_sta,
			   struct ieee80211_tx_rate_control *txrc)
{
[...]
	if (!pubsta || !priv_sta || rc_no_data_or_no_ack_use_min(txrc)) {
[...]
		return true;
	}
[...]
}


Which means that if priv_sta (aka mvm_sta) is NULL, we will return
without running the rest of rs_get_rate() where lq_sta is accessed
without checks.

I admit that the rs_get_rate() function is a bit hard to read, but
removing the lq_sta check as you did doesn't help, but makes things
worse.

--
Cheers,
Luca.