From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: 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 ); Thu, 19 May 2016 04:27:19 -0400 From: "Coelho, Luciano" To: "kvalo@codeaurora.org" , "xypron.glpk@gmx.de" , "Berg, Johannes" , "Grumbach, Emmanuel" CC: linuxwifi , "eyal@wizery.com" , "netdev@vger.kernel.org" , "linux-wireless@vger.kernel.org" , "Greenman, Gregory" , "Bondar, Alexander" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 1/1] iwlwifi: rs: remove superfluous check Date: Thu, 19 May 2016 08:27:13 +0000 Message-ID: <1463646433.29999.50.camel@intel.com> (sfid-20160519_102749_261496_7FA48233) References: <1463527868-12226-1-git-send-email-xypron.glpk@gmx.de> In-Reply-To: <1463527868-12226-1-git-send-email-xypron.glpk@gmx.de> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: T24gV2VkLCAyMDE2LTA1LTE4IGF0IDAxOjMxICswMjAwLCBIZWlucmljaCBTY2h1Y2hhcmR0IHdy b3RlOg0KPiBJZiB3ZSBkZXJlZmVyZW5jZSBhIHZhcmlhYmxlIGFueXdheSBpbiBvdGhlciBwYXJ0 cyBvZiB0aGUgY29kZSwNCj4gdGhlcmUgaXMgbm8gbmVlZCB0byBjaGVjayBhZ2FpbnN0IE5VTEwg aW4gYSBzaW5nbGUgcGxhY2UuDQoNCk5BQ0suIMKgVGhpcyBpcyBub3QgdHJ1ZS4NCg0KSWYgbHFf c3RhIGlzIE5VTEwsIGl0IG1lYW5zIHRoYXQgbXZtX3N0YSBpcyBhbHNvIE5VTEwuwqDCoFRoZW4g d2UgY2FsbA0KdGhlIHJhdGVfY29udHJvbF9zZW5kIHdpdGggbXZtX3N0YSA9PSBOVUxMOg0KDQoJ aWYgKHJhdGVfY29udHJvbF9zZW5kX2xvdyhzdGEsIG12bV9zdGEsIHR4cmMpKQ0KCQlyZXR1cm47 DQoNClRoZSByYXRlX2NvbnRyb2xfc2VuZF9sb3coKSBmdW5jdGlvbiBsb29rcyBsaWtlIHRoaXM6 DQoNCg0KYm9vbCByYXRlX2NvbnRyb2xfc2VuZF9sb3coc3RydWN0IGllZWU4MDIxMV9zdGEgKnB1 YnN0YSwNCgkJCcKgwqDCoHZvaWQgKnByaXZfc3RhLA0KCQkJwqDCoMKgc3RydWN0IGllZWU4MDIx MV90eF9yYXRlX2NvbnRyb2wgKnR4cmMpDQp7DQpbLi4uXQ0KCWlmICghcHVic3RhIHx8ICFwcml2 X3N0YSB8fCByY19ub19kYXRhX29yX25vX2Fja191c2VfbWluKHR4cmMpKSB7DQpbLi4uXQ0KCQly ZXR1cm4gdHJ1ZTsNCgl9DQpbLi4uXQ0KfQ0KDQoNCldoaWNoIG1lYW5zIHRoYXQgaWYgcHJpdl9z dGEgKGFrYSBtdm1fc3RhKSBpcyBOVUxMLCB3ZSB3aWxsIHJldHVybg0Kd2l0aG91dCBydW5uaW5n IHRoZSByZXN0IG9mIHJzX2dldF9yYXRlKCkgd2hlcmUgbHFfc3RhIGlzIGFjY2Vzc2VkDQp3aXRo b3V0IGNoZWNrcy4NCg0KSSBhZG1pdCB0aGF0IHRoZSByc19nZXRfcmF0ZSgpIGZ1bmN0aW9uIGlz IGEgYml0IGhhcmQgdG8gcmVhZCwgYnV0DQpyZW1vdmluZyB0aGUgbHFfc3RhIGNoZWNrIGFzIHlv dSBkaWQgZG9lc24ndCBoZWxwLCBidXQgbWFrZXMgdGhpbmdzDQp3b3JzZS4NCg0KLS0NCkNoZWVy cywNCkx1Y2Eu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752344AbcESI1X (ORCPT ); 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 ); 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" To: "kvalo@codeaurora.org" , "xypron.glpk@gmx.de" , "Berg, Johannes" , "Grumbach, Emmanuel" CC: linuxwifi , "eyal@wizery.com" , "netdev@vger.kernel.org" , "linux-wireless@vger.kernel.org" , "Greenman, Gregory" , "Bondar, Alexander" , "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: 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.