diff for duplicates of <1463646433.29999.50.camel@intel.com> diff --git a/a/1.txt b/N1/1.txt index 21774ad..fc363da 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,19 +1,39 @@ -T24gV2VkLCAyMDE2LTA1LTE4IGF0IDAxOjMxICswMjAwLCBIZWlucmljaCBTY2h1Y2hhcmR0IHdy -b3RlOg0KPiBJZiB3ZSBkZXJlZmVyZW5jZSBhIHZhcmlhYmxlIGFueXdheSBpbiBvdGhlciBwYXJ0 -cyBvZiB0aGUgY29kZSwNCj4gdGhlcmUgaXMgbm8gbmVlZCB0byBjaGVjayBhZ2FpbnN0IE5VTEwg -aW4gYSBzaW5nbGUgcGxhY2UuDQoNCk5BQ0suIMKgVGhpcyBpcyBub3QgdHJ1ZS4NCg0KSWYgbHFf -c3RhIGlzIE5VTEwsIGl0IG1lYW5zIHRoYXQgbXZtX3N0YSBpcyBhbHNvIE5VTEwuwqDCoFRoZW4g -d2UgY2FsbA0KdGhlIHJhdGVfY29udHJvbF9zZW5kIHdpdGggbXZtX3N0YSA9PSBOVUxMOg0KDQoJ -aWYgKHJhdGVfY29udHJvbF9zZW5kX2xvdyhzdGEsIG12bV9zdGEsIHR4cmMpKQ0KCQlyZXR1cm47 -DQoNClRoZSByYXRlX2NvbnRyb2xfc2VuZF9sb3coKSBmdW5jdGlvbiBsb29rcyBsaWtlIHRoaXM6 -DQoNCg0KYm9vbCByYXRlX2NvbnRyb2xfc2VuZF9sb3coc3RydWN0IGllZWU4MDIxMV9zdGEgKnB1 -YnN0YSwNCgkJCcKgwqDCoHZvaWQgKnByaXZfc3RhLA0KCQkJwqDCoMKgc3RydWN0IGllZWU4MDIx -MV90eF9yYXRlX2NvbnRyb2wgKnR4cmMpDQp7DQpbLi4uXQ0KCWlmICghcHVic3RhIHx8ICFwcml2 -X3N0YSB8fCByY19ub19kYXRhX29yX25vX2Fja191c2VfbWluKHR4cmMpKSB7DQpbLi4uXQ0KCQly -ZXR1cm4gdHJ1ZTsNCgl9DQpbLi4uXQ0KfQ0KDQoNCldoaWNoIG1lYW5zIHRoYXQgaWYgcHJpdl9z -dGEgKGFrYSBtdm1fc3RhKSBpcyBOVUxMLCB3ZSB3aWxsIHJldHVybg0Kd2l0aG91dCBydW5uaW5n -IHRoZSByZXN0IG9mIHJzX2dldF9yYXRlKCkgd2hlcmUgbHFfc3RhIGlzIGFjY2Vzc2VkDQp3aXRo -b3V0IGNoZWNrcy4NCg0KSSBhZG1pdCB0aGF0IHRoZSByc19nZXRfcmF0ZSgpIGZ1bmN0aW9uIGlz -IGEgYml0IGhhcmQgdG8gcmVhZCwgYnV0DQpyZW1vdmluZyB0aGUgbHFfc3RhIGNoZWNrIGFzIHlv -dSBkaWQgZG9lc24ndCBoZWxwLCBidXQgbWFrZXMgdGhpbmdzDQp3b3JzZS4NCg0KLS0NCkNoZWVy -cywNCkx1Y2Eu +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. diff --git a/a/content_digest b/N1/content_digest index a42876a..3966589 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -19,24 +19,44 @@ " linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>\0" "\00:1\0" "b\0" - "T24gV2VkLCAyMDE2LTA1LTE4IGF0IDAxOjMxICswMjAwLCBIZWlucmljaCBTY2h1Y2hhcmR0IHdy\n" - "b3RlOg0KPiBJZiB3ZSBkZXJlZmVyZW5jZSBhIHZhcmlhYmxlIGFueXdheSBpbiBvdGhlciBwYXJ0\n" - "cyBvZiB0aGUgY29kZSwNCj4gdGhlcmUgaXMgbm8gbmVlZCB0byBjaGVjayBhZ2FpbnN0IE5VTEwg\n" - "aW4gYSBzaW5nbGUgcGxhY2UuDQoNCk5BQ0suIMKgVGhpcyBpcyBub3QgdHJ1ZS4NCg0KSWYgbHFf\n" - "c3RhIGlzIE5VTEwsIGl0IG1lYW5zIHRoYXQgbXZtX3N0YSBpcyBhbHNvIE5VTEwuwqDCoFRoZW4g\n" - "d2UgY2FsbA0KdGhlIHJhdGVfY29udHJvbF9zZW5kIHdpdGggbXZtX3N0YSA9PSBOVUxMOg0KDQoJ\n" - "aWYgKHJhdGVfY29udHJvbF9zZW5kX2xvdyhzdGEsIG12bV9zdGEsIHR4cmMpKQ0KCQlyZXR1cm47\n" - "DQoNClRoZSByYXRlX2NvbnRyb2xfc2VuZF9sb3coKSBmdW5jdGlvbiBsb29rcyBsaWtlIHRoaXM6\n" - "DQoNCg0KYm9vbCByYXRlX2NvbnRyb2xfc2VuZF9sb3coc3RydWN0IGllZWU4MDIxMV9zdGEgKnB1\n" - "YnN0YSwNCgkJCcKgwqDCoHZvaWQgKnByaXZfc3RhLA0KCQkJwqDCoMKgc3RydWN0IGllZWU4MDIx\n" - "MV90eF9yYXRlX2NvbnRyb2wgKnR4cmMpDQp7DQpbLi4uXQ0KCWlmICghcHVic3RhIHx8ICFwcml2\n" - "X3N0YSB8fCByY19ub19kYXRhX29yX25vX2Fja191c2VfbWluKHR4cmMpKSB7DQpbLi4uXQ0KCQly\n" - "ZXR1cm4gdHJ1ZTsNCgl9DQpbLi4uXQ0KfQ0KDQoNCldoaWNoIG1lYW5zIHRoYXQgaWYgcHJpdl9z\n" - "dGEgKGFrYSBtdm1fc3RhKSBpcyBOVUxMLCB3ZSB3aWxsIHJldHVybg0Kd2l0aG91dCBydW5uaW5n\n" - "IHRoZSByZXN0IG9mIHJzX2dldF9yYXRlKCkgd2hlcmUgbHFfc3RhIGlzIGFjY2Vzc2VkDQp3aXRo\n" - "b3V0IGNoZWNrcy4NCg0KSSBhZG1pdCB0aGF0IHRoZSByc19nZXRfcmF0ZSgpIGZ1bmN0aW9uIGlz\n" - "IGEgYml0IGhhcmQgdG8gcmVhZCwgYnV0DQpyZW1vdmluZyB0aGUgbHFfc3RhIGNoZWNrIGFzIHlv\n" - "dSBkaWQgZG9lc24ndCBoZWxwLCBidXQgbWFrZXMgdGhpbmdzDQp3b3JzZS4NCg0KLS0NCkNoZWVy\n" - cywNCkx1Y2Eu + "On Wed, 2016-05-18 at 01:31 +0200, Heinrich Schuchardt wrote:\n" + "> If we dereference a variable anyway in other parts of the code,\n" + "> there is no need to check against NULL in a single place.\n" + "\n" + "NACK. \302\240This is not true.\n" + "\n" + "If lq_sta is NULL, it means that mvm_sta is also NULL.\302\240\302\240Then we call\n" + "the rate_control_send with mvm_sta == NULL:\n" + "\n" + "\tif (rate_control_send_low(sta, mvm_sta, txrc))\n" + "\t\treturn;\n" + "\n" + "The rate_control_send_low() function looks like this:\n" + "\n" + "\n" + "bool rate_control_send_low(struct ieee80211_sta *pubsta,\n" + "\t\t\t\302\240\302\240\302\240void *priv_sta,\n" + "\t\t\t\302\240\302\240\302\240struct ieee80211_tx_rate_control *txrc)\n" + "{\n" + "[...]\n" + "\tif (!pubsta || !priv_sta || rc_no_data_or_no_ack_use_min(txrc)) {\n" + "[...]\n" + "\t\treturn true;\n" + "\t}\n" + "[...]\n" + "}\n" + "\n" + "\n" + "Which means that if priv_sta (aka mvm_sta) is NULL, we will return\n" + "without running the rest of rs_get_rate() where lq_sta is accessed\n" + "without checks.\n" + "\n" + "I admit that the rs_get_rate() function is a bit hard to read, but\n" + "removing the lq_sta check as you did doesn't help, but makes things\n" + "worse.\n" + "\n" + "--\n" + "Cheers,\n" + Luca. -dca9628e826662458d238fe15b5bc38e805efde6b48209967afb815038d7d799 +b50bcc3b5a2d034066f225b3f64124de84d8b661606568794b2d1479ed4f6bef
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.