diff for duplicates of <1433956209.20602.5.camel@intel.com> diff --git a/a/1.txt b/N1/1.txt index 3b729bb..f778029 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,39 +1,55 @@ -T24gV2VkLCAyMDE1LTA2LTEwIGF0IDEyOjU4IC0wNDAwLCBOaWNob2xhcyBLcmF1c2Ugd3JvdGU6 -DQo+IA0KPiBPbiBKdW5lIDEwLCAyMDE1IDEyOjUwOjQ1IFBNIEVEVCwgIkdydW1iYWNoLCBFbW1h -bnVlbCIgPGVtbWFudWVsLmdydW1iYWNoQGludGVsLmNvbT4gd3JvdGU6DQo+ID5PbiBXZWQsIDIw -MTUtMDYtMTAgYXQgMTI6MzMgLTA0MDAsIE5pY2hvbGFzIEtyYXVzZSB3cm90ZToNCj4gPj4gVGhp -cyBtYWtlcyB0aGUgZnVuY3Rpb24gaXdsX3Jlc3VtZV9zdGF0dXNfZm4gcmV0dXJuIGZhbHNlIG5v -dyBpZg0KPiA+PiB0aGUgcmVjZWl2ZWQgcGFja2V0IG9mIHR5cGUgaXdsX3J4X3BhY2tldCBpcyBu -b3QgdGhlIHNhbWUgc2l6ZQ0KPiA+PiBhcyB0aGUgc3RydWN0dXJlIHBvaW50ZXIsIGl3bF9yZXN1 -bWVfZGF0YSdzIGNtZCBlbGVtZW50IGluIG9yZGVyDQo+ID4+IHRvIHNpZ25hbCBjYWxsZXJzIGFi -b3V0IHRoaXMgZXJyb3IgYW5kIGFsbG93IHRoZW0gdG8gaGFuZGxlIGl0DQo+ID4+IG9jY3VycmVu -dGx5Lg0KPiA+PiANCj4gPg0KPiA+SG0uLi4gRGlkIHlvdSBhY3R1YWxseSBoaXQgdGhpcyBpZj8N -Cj4gPkkgYW0gbm90IHN1cmUgSSByZWFsbHkgd2FudCB0byB3YWl0IGhlcmUgKHdoaWNoIGlzIHdo -YXQgd2lsbCBoYXBwZW4gaWYNCj4gPnlvdSByZXR1cm4gZmFsc2UpIHdoZW4gd2UgZ2V0IGFuIHVu -ZXhwZWN0ZWQgbGVuZ3RoPyBJIGRvIG5vdCBleHBlY3QNCj4gPmFueXRoaW5nIGJlc2lkZXMgdGhl -IHJlc3BvbnNlIEkgYW0gd2FpdGluZyBmb3Igc2luY2UgdGhlIGZpcm13YXJlIGlzDQo+ID5oYW5k -bGluZyB0aGUgR0VUX1NUQVRVUyAqb25seSogLSBpdCBqdXN0IGNhbWUgYmFjayBmcm9tIFdvV0xB -Ti4gQm90dG9tDQo+ID5saW5lLCB0aGlzIGlzIHJlYWxseSBhbiBlcnJvciBwYXRoIGFuZCBJIHBy -ZWZlciB0byBleGl0IGFuZCBub3Qgd2FpdA0KPiA+Zm9yDQo+ID50aGUgdGltZW91dCBpbiB0aGF0 -IGNhc2UuDQo+ID5CdXQgSSBtaWdodCBiZSBtaXNzaW5nIHNvbWV0aGluZz8NCj4gPg0KPiBXaHkg -bm90IHdhaXQgZm9yIHRoZSB0aW1lIG91dD8gIFNlZW1zIHRoZXJlIGlzIG5vIHJlYXNvbiBub3Qg -dG8gYW5kIGluIHRoYXQgY2FzZQ0KPiBpZiB0aGUgZmlybXdhcmUgaGFuZGxlcyB0aGlzDQoNCkkg -ZG91YnQgaXQgd2lsbC4gVGhpcyBnb2VzIGJhY2sgdG8gbXkgb3JpZ2luYWwgcXVlc3Rpb246IGRp -ZCB5b3UgcmVhbGx5DQpoaXQgdGhpcyBwYXRoPyBEb2VzIHRoZSBwYXRjaCBzb2x2ZXMgYSByZWFs -IGJ1ZyB5b3UgZmFjZWQ/DQoNCj4gd2h5IGlzIHRoZSBpZiBzdGF0ZW1lbnQgc3RpbGwgaGVyZS4g -DQoNClRvIGdldCBhIGRlYnVnIHByaW50PyA6KQ0KDQo+IE5pY2sgDQo+ID4+IFNpZ25lZC1vZmYt -Ynk6IE5pY2hvbGFzIEtyYXVzZSA8eGVyb2ZvaWZ5QGdtYWlsLmNvbT4NCj4gPj4gLS0tDQo+ID4+ -ICBkcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jIHwgMiArLQ0KPiA+ -PiAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4+IA0K -PiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAy -MTEuYw0KPiA+Yi9kcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jDQo+ -ID4+IGluZGV4IDVhYmQ2MmUuLjIxZTgwOGMgMTAwNjQ0DQo+ID4+IC0tLSBhL2RyaXZlcnMvbmV0 -L3dpcmVsZXNzL2l3bHdpZmkvZHZtL21hYzgwMjExLmMNCj4gPj4gKysrIGIvZHJpdmVycy9uZXQv -d2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAyMTEuYw0KPiA+PiBAQCAtNDA5LDcgKzQwOSw3IEBA -IHN0YXRpYyBib29sIGl3bF9yZXN1bWVfc3RhdHVzX2ZuKHN0cnVjdA0KPiA+aXdsX25vdGlmX3dh -aXRfZGF0YSAqbm90aWZfd2FpdCwNCj4gPj4gIA0KPiA+PiAgCWlmIChpd2xfcnhfcGFja2V0X3Bh -eWxvYWRfbGVuKHBrdCkgIT0gc2l6ZW9mKCpyZXN1bWVfZGF0YS0+Y21kKSkgew0KPiA+PiAgCQlJ -V0xfRVJSKHByaXYsICJyeCB3cm9uZyBzaXplIGRhdGFcbiIpOw0KPiA+PiAtCQlyZXR1cm4gdHJ1 -ZTsNCj4gPj4gKwkJcmV0dXJuIGZhbHNlOw0KPiA+PiAgCX0NCj4gPj4gIAltZW1jcHkocmVzdW1l -X2RhdGEtPmNtZCwgcGt0LT5kYXRhLCBzaXplb2YoKnJlc3VtZV9kYXRhLT5jbWQpKTsNCj4gPj4g -IAlyZXN1bWVfZGF0YS0+dmFsaWQgPSB0cnVlOw0KPiANCg0K +On Wed, 2015-06-10 at 12:58 -0400, Nicholas Krause wrote: +> +> On June 10, 2015 12:50:45 PM EDT, "Grumbach, Emmanuel" <emmanuel.grumbach@intel.com> wrote: +> >On Wed, 2015-06-10 at 12:33 -0400, Nicholas Krause wrote: +> >> This makes the function iwl_resume_status_fn return false now if +> >> the received packet of type iwl_rx_packet is not the same size +> >> as the structure pointer, iwl_resume_data's cmd element in order +> >> to signal callers about this error and allow them to handle it +> >> occurrently. +> >> +> > +> >Hm... Did you actually hit this if? +> >I am not sure I really want to wait here (which is what will happen if +> >you return false) when we get an unexpected length? I do not expect +> >anything besides the response I am waiting for since the firmware is +> >handling the GET_STATUS *only* - it just came back from WoWLAN. Bottom +> >line, this is really an error path and I prefer to exit and not wait +> >for +> >the timeout in that case. +> >But I might be missing something? +> > +> Why not wait for the time out? Seems there is no reason not to and in that case +> if the firmware handles this + +I doubt it will. This goes back to my original question: did you really +hit this path? Does the patch solves a real bug you faced? + +> why is the if statement still here. + +To get a debug print? :) + +> Nick +> >> Signed-off-by: Nicholas Krause <xerofoify@gmail.com> +> >> --- +> >> drivers/net/wireless/iwlwifi/dvm/mac80211.c | 2 +- +> >> 1 file changed, 1 insertion(+), 1 deletion(-) +> >> +> >> diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >b/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >> index 5abd62e..21e808c 100644 +> >> --- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >> +++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >> @@ -409,7 +409,7 @@ static bool iwl_resume_status_fn(struct +> >iwl_notif_wait_data *notif_wait, +> >> +> >> if (iwl_rx_packet_payload_len(pkt) != sizeof(*resume_data->cmd)) { +> >> IWL_ERR(priv, "rx wrong size data\n"); +> >> - return true; +> >> + return false; +> >> } +> >> memcpy(resume_data->cmd, pkt->data, sizeof(*resume_data->cmd)); +> >> resume_data->valid = true; +> + +ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥ diff --git a/a/content_digest b/N1/content_digest index 986d31c..6c5db43 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -19,44 +19,60 @@ " linux-wireless@vger.kernel.org <linux-wireless@vger.kernel.org>\0" "\00:1\0" "b\0" - "T24gV2VkLCAyMDE1LTA2LTEwIGF0IDEyOjU4IC0wNDAwLCBOaWNob2xhcyBLcmF1c2Ugd3JvdGU6\n" - "DQo+IA0KPiBPbiBKdW5lIDEwLCAyMDE1IDEyOjUwOjQ1IFBNIEVEVCwgIkdydW1iYWNoLCBFbW1h\n" - "bnVlbCIgPGVtbWFudWVsLmdydW1iYWNoQGludGVsLmNvbT4gd3JvdGU6DQo+ID5PbiBXZWQsIDIw\n" - "MTUtMDYtMTAgYXQgMTI6MzMgLTA0MDAsIE5pY2hvbGFzIEtyYXVzZSB3cm90ZToNCj4gPj4gVGhp\n" - "cyBtYWtlcyB0aGUgZnVuY3Rpb24gaXdsX3Jlc3VtZV9zdGF0dXNfZm4gcmV0dXJuIGZhbHNlIG5v\n" - "dyBpZg0KPiA+PiB0aGUgcmVjZWl2ZWQgcGFja2V0IG9mIHR5cGUgaXdsX3J4X3BhY2tldCBpcyBu\n" - "b3QgdGhlIHNhbWUgc2l6ZQ0KPiA+PiBhcyB0aGUgc3RydWN0dXJlIHBvaW50ZXIsIGl3bF9yZXN1\n" - "bWVfZGF0YSdzIGNtZCBlbGVtZW50IGluIG9yZGVyDQo+ID4+IHRvIHNpZ25hbCBjYWxsZXJzIGFi\n" - "b3V0IHRoaXMgZXJyb3IgYW5kIGFsbG93IHRoZW0gdG8gaGFuZGxlIGl0DQo+ID4+IG9jY3VycmVu\n" - "dGx5Lg0KPiA+PiANCj4gPg0KPiA+SG0uLi4gRGlkIHlvdSBhY3R1YWxseSBoaXQgdGhpcyBpZj8N\n" - "Cj4gPkkgYW0gbm90IHN1cmUgSSByZWFsbHkgd2FudCB0byB3YWl0IGhlcmUgKHdoaWNoIGlzIHdo\n" - "YXQgd2lsbCBoYXBwZW4gaWYNCj4gPnlvdSByZXR1cm4gZmFsc2UpIHdoZW4gd2UgZ2V0IGFuIHVu\n" - "ZXhwZWN0ZWQgbGVuZ3RoPyBJIGRvIG5vdCBleHBlY3QNCj4gPmFueXRoaW5nIGJlc2lkZXMgdGhl\n" - "IHJlc3BvbnNlIEkgYW0gd2FpdGluZyBmb3Igc2luY2UgdGhlIGZpcm13YXJlIGlzDQo+ID5oYW5k\n" - "bGluZyB0aGUgR0VUX1NUQVRVUyAqb25seSogLSBpdCBqdXN0IGNhbWUgYmFjayBmcm9tIFdvV0xB\n" - "Ti4gQm90dG9tDQo+ID5saW5lLCB0aGlzIGlzIHJlYWxseSBhbiBlcnJvciBwYXRoIGFuZCBJIHBy\n" - "ZWZlciB0byBleGl0IGFuZCBub3Qgd2FpdA0KPiA+Zm9yDQo+ID50aGUgdGltZW91dCBpbiB0aGF0\n" - "IGNhc2UuDQo+ID5CdXQgSSBtaWdodCBiZSBtaXNzaW5nIHNvbWV0aGluZz8NCj4gPg0KPiBXaHkg\n" - "bm90IHdhaXQgZm9yIHRoZSB0aW1lIG91dD8gIFNlZW1zIHRoZXJlIGlzIG5vIHJlYXNvbiBub3Qg\n" - "dG8gYW5kIGluIHRoYXQgY2FzZQ0KPiBpZiB0aGUgZmlybXdhcmUgaGFuZGxlcyB0aGlzDQoNCkkg\n" - "ZG91YnQgaXQgd2lsbC4gVGhpcyBnb2VzIGJhY2sgdG8gbXkgb3JpZ2luYWwgcXVlc3Rpb246IGRp\n" - "ZCB5b3UgcmVhbGx5DQpoaXQgdGhpcyBwYXRoPyBEb2VzIHRoZSBwYXRjaCBzb2x2ZXMgYSByZWFs\n" - "IGJ1ZyB5b3UgZmFjZWQ/DQoNCj4gd2h5IGlzIHRoZSBpZiBzdGF0ZW1lbnQgc3RpbGwgaGVyZS4g\n" - "DQoNClRvIGdldCBhIGRlYnVnIHByaW50PyA6KQ0KDQo+IE5pY2sgDQo+ID4+IFNpZ25lZC1vZmYt\n" - "Ynk6IE5pY2hvbGFzIEtyYXVzZSA8eGVyb2ZvaWZ5QGdtYWlsLmNvbT4NCj4gPj4gLS0tDQo+ID4+\n" - "ICBkcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jIHwgMiArLQ0KPiA+\n" - "PiAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4+IA0K\n" - "PiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAy\n" - "MTEuYw0KPiA+Yi9kcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jDQo+\n" - "ID4+IGluZGV4IDVhYmQ2MmUuLjIxZTgwOGMgMTAwNjQ0DQo+ID4+IC0tLSBhL2RyaXZlcnMvbmV0\n" - "L3dpcmVsZXNzL2l3bHdpZmkvZHZtL21hYzgwMjExLmMNCj4gPj4gKysrIGIvZHJpdmVycy9uZXQv\n" - "d2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAyMTEuYw0KPiA+PiBAQCAtNDA5LDcgKzQwOSw3IEBA\n" - "IHN0YXRpYyBib29sIGl3bF9yZXN1bWVfc3RhdHVzX2ZuKHN0cnVjdA0KPiA+aXdsX25vdGlmX3dh\n" - "aXRfZGF0YSAqbm90aWZfd2FpdCwNCj4gPj4gIA0KPiA+PiAgCWlmIChpd2xfcnhfcGFja2V0X3Bh\n" - "eWxvYWRfbGVuKHBrdCkgIT0gc2l6ZW9mKCpyZXN1bWVfZGF0YS0+Y21kKSkgew0KPiA+PiAgCQlJ\n" - "V0xfRVJSKHByaXYsICJyeCB3cm9uZyBzaXplIGRhdGFcbiIpOw0KPiA+PiAtCQlyZXR1cm4gdHJ1\n" - "ZTsNCj4gPj4gKwkJcmV0dXJuIGZhbHNlOw0KPiA+PiAgCX0NCj4gPj4gIAltZW1jcHkocmVzdW1l\n" - "X2RhdGEtPmNtZCwgcGt0LT5kYXRhLCBzaXplb2YoKnJlc3VtZV9kYXRhLT5jbWQpKTsNCj4gPj4g\n" - IAlyZXN1bWVfZGF0YS0+dmFsaWQgPSB0cnVlOw0KPiANCg0K + "On Wed, 2015-06-10 at 12:58 -0400, Nicholas Krause wrote:\n" + "> \n" + "> On June 10, 2015 12:50:45 PM EDT, \"Grumbach, Emmanuel\" <emmanuel.grumbach@intel.com> wrote:\n" + "> >On Wed, 2015-06-10 at 12:33 -0400, Nicholas Krause wrote:\n" + "> >> This makes the function iwl_resume_status_fn return false now if\n" + "> >> the received packet of type iwl_rx_packet is not the same size\n" + "> >> as the structure pointer, iwl_resume_data's cmd element in order\n" + "> >> to signal callers about this error and allow them to handle it\n" + "> >> occurrently.\n" + "> >> \n" + "> >\n" + "> >Hm... Did you actually hit this if?\n" + "> >I am not sure I really want to wait here (which is what will happen if\n" + "> >you return false) when we get an unexpected length? I do not expect\n" + "> >anything besides the response I am waiting for since the firmware is\n" + "> >handling the GET_STATUS *only* - it just came back from WoWLAN. Bottom\n" + "> >line, this is really an error path and I prefer to exit and not wait\n" + "> >for\n" + "> >the timeout in that case.\n" + "> >But I might be missing something?\n" + "> >\n" + "> Why not wait for the time out? Seems there is no reason not to and in that case\n" + "> if the firmware handles this\n" + "\n" + "I doubt it will. This goes back to my original question: did you really\n" + "hit this path? Does the patch solves a real bug you faced?\n" + "\n" + "> why is the if statement still here. \n" + "\n" + "To get a debug print? :)\n" + "\n" + "> Nick \n" + "> >> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>\n" + "> >> ---\n" + "> >> drivers/net/wireless/iwlwifi/dvm/mac80211.c | 2 +-\n" + "> >> 1 file changed, 1 insertion(+), 1 deletion(-)\n" + "> >> \n" + "> >> diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >b/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >> index 5abd62e..21e808c 100644\n" + "> >> --- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >> +++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >> @@ -409,7 +409,7 @@ static bool iwl_resume_status_fn(struct\n" + "> >iwl_notif_wait_data *notif_wait,\n" + "> >> \n" + "> >> \tif (iwl_rx_packet_payload_len(pkt) != sizeof(*resume_data->cmd)) {\n" + "> >> \t\tIWL_ERR(priv, \"rx wrong size data\\n\");\n" + "> >> -\t\treturn true;\n" + "> >> +\t\treturn false;\n" + "> >> \t}\n" + "> >> \tmemcpy(resume_data->cmd, pkt->data, sizeof(*resume_data->cmd));\n" + "> >> \tresume_data->valid = true;\n" + "> \n" + "\n" + "\303\277\303\264\303\250\302\272{.n\303\207+\302\211\302\267\302\237\302\256\302\211\302\255\302\206+%\302\212\303\213\303\277\302\261\303\251\303\235\302\266\027\302\245\302\212w\303\277\302\272{.n\303\207+\302\211\302\267\302\245\302\212{\302\261\303\276G\302\253\302\235\303\251\303\277\302\212{ay\302\272\035\303\212\302\207\303\232\302\231\303\253,j\a\302\255\302\242f\302\243\302\242\302\267h\302\232\302\217\303\257\302\201\303\252\303\277\302\221\303\252\303\247z_\303\250\302\256\003(\302\255\303\251\302\232\302\216\302\212\303\235\302\242j\"\302\235\303\272\032\302\266\033m\302\247\303\277\303\277\302\276\a\302\253\303\276G\302\253\302\235\303\251\303\277\302\242\302\270?\302\231\302\250\303\250\302\255\303\232&\302\243\303\270\302\247~\302\217\303\241\302\266iO\302\225\303\246\302\254z\302\267\302\232v\303\230^\024\004\032\302\266\033m\302\247\303\277\303\277\303\203\f\303\277\302\266\303\254\303\277\302\242\302\270?\302\226I\302\245" -0ca522a11c8d985142d94dc8dda2668bce5860ce71d545ddf3f9d8b499045ed1 +58a438721994691c10d16caa3cf124841b8c6d5b21ff37dc83fc6007ac01ad9c
diff --git a/a/1.txt b/N2/1.txt index 3b729bb..6ef6f1b 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -1,39 +1,53 @@ -T24gV2VkLCAyMDE1LTA2LTEwIGF0IDEyOjU4IC0wNDAwLCBOaWNob2xhcyBLcmF1c2Ugd3JvdGU6 -DQo+IA0KPiBPbiBKdW5lIDEwLCAyMDE1IDEyOjUwOjQ1IFBNIEVEVCwgIkdydW1iYWNoLCBFbW1h -bnVlbCIgPGVtbWFudWVsLmdydW1iYWNoQGludGVsLmNvbT4gd3JvdGU6DQo+ID5PbiBXZWQsIDIw -MTUtMDYtMTAgYXQgMTI6MzMgLTA0MDAsIE5pY2hvbGFzIEtyYXVzZSB3cm90ZToNCj4gPj4gVGhp -cyBtYWtlcyB0aGUgZnVuY3Rpb24gaXdsX3Jlc3VtZV9zdGF0dXNfZm4gcmV0dXJuIGZhbHNlIG5v -dyBpZg0KPiA+PiB0aGUgcmVjZWl2ZWQgcGFja2V0IG9mIHR5cGUgaXdsX3J4X3BhY2tldCBpcyBu -b3QgdGhlIHNhbWUgc2l6ZQ0KPiA+PiBhcyB0aGUgc3RydWN0dXJlIHBvaW50ZXIsIGl3bF9yZXN1 -bWVfZGF0YSdzIGNtZCBlbGVtZW50IGluIG9yZGVyDQo+ID4+IHRvIHNpZ25hbCBjYWxsZXJzIGFi -b3V0IHRoaXMgZXJyb3IgYW5kIGFsbG93IHRoZW0gdG8gaGFuZGxlIGl0DQo+ID4+IG9jY3VycmVu -dGx5Lg0KPiA+PiANCj4gPg0KPiA+SG0uLi4gRGlkIHlvdSBhY3R1YWxseSBoaXQgdGhpcyBpZj8N -Cj4gPkkgYW0gbm90IHN1cmUgSSByZWFsbHkgd2FudCB0byB3YWl0IGhlcmUgKHdoaWNoIGlzIHdo -YXQgd2lsbCBoYXBwZW4gaWYNCj4gPnlvdSByZXR1cm4gZmFsc2UpIHdoZW4gd2UgZ2V0IGFuIHVu -ZXhwZWN0ZWQgbGVuZ3RoPyBJIGRvIG5vdCBleHBlY3QNCj4gPmFueXRoaW5nIGJlc2lkZXMgdGhl -IHJlc3BvbnNlIEkgYW0gd2FpdGluZyBmb3Igc2luY2UgdGhlIGZpcm13YXJlIGlzDQo+ID5oYW5k -bGluZyB0aGUgR0VUX1NUQVRVUyAqb25seSogLSBpdCBqdXN0IGNhbWUgYmFjayBmcm9tIFdvV0xB -Ti4gQm90dG9tDQo+ID5saW5lLCB0aGlzIGlzIHJlYWxseSBhbiBlcnJvciBwYXRoIGFuZCBJIHBy -ZWZlciB0byBleGl0IGFuZCBub3Qgd2FpdA0KPiA+Zm9yDQo+ID50aGUgdGltZW91dCBpbiB0aGF0 -IGNhc2UuDQo+ID5CdXQgSSBtaWdodCBiZSBtaXNzaW5nIHNvbWV0aGluZz8NCj4gPg0KPiBXaHkg -bm90IHdhaXQgZm9yIHRoZSB0aW1lIG91dD8gIFNlZW1zIHRoZXJlIGlzIG5vIHJlYXNvbiBub3Qg -dG8gYW5kIGluIHRoYXQgY2FzZQ0KPiBpZiB0aGUgZmlybXdhcmUgaGFuZGxlcyB0aGlzDQoNCkkg -ZG91YnQgaXQgd2lsbC4gVGhpcyBnb2VzIGJhY2sgdG8gbXkgb3JpZ2luYWwgcXVlc3Rpb246IGRp -ZCB5b3UgcmVhbGx5DQpoaXQgdGhpcyBwYXRoPyBEb2VzIHRoZSBwYXRjaCBzb2x2ZXMgYSByZWFs -IGJ1ZyB5b3UgZmFjZWQ/DQoNCj4gd2h5IGlzIHRoZSBpZiBzdGF0ZW1lbnQgc3RpbGwgaGVyZS4g -DQoNClRvIGdldCBhIGRlYnVnIHByaW50PyA6KQ0KDQo+IE5pY2sgDQo+ID4+IFNpZ25lZC1vZmYt -Ynk6IE5pY2hvbGFzIEtyYXVzZSA8eGVyb2ZvaWZ5QGdtYWlsLmNvbT4NCj4gPj4gLS0tDQo+ID4+ -ICBkcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jIHwgMiArLQ0KPiA+ -PiAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4+IA0K -PiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAy -MTEuYw0KPiA+Yi9kcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jDQo+ -ID4+IGluZGV4IDVhYmQ2MmUuLjIxZTgwOGMgMTAwNjQ0DQo+ID4+IC0tLSBhL2RyaXZlcnMvbmV0 -L3dpcmVsZXNzL2l3bHdpZmkvZHZtL21hYzgwMjExLmMNCj4gPj4gKysrIGIvZHJpdmVycy9uZXQv -d2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAyMTEuYw0KPiA+PiBAQCAtNDA5LDcgKzQwOSw3IEBA -IHN0YXRpYyBib29sIGl3bF9yZXN1bWVfc3RhdHVzX2ZuKHN0cnVjdA0KPiA+aXdsX25vdGlmX3dh -aXRfZGF0YSAqbm90aWZfd2FpdCwNCj4gPj4gIA0KPiA+PiAgCWlmIChpd2xfcnhfcGFja2V0X3Bh -eWxvYWRfbGVuKHBrdCkgIT0gc2l6ZW9mKCpyZXN1bWVfZGF0YS0+Y21kKSkgew0KPiA+PiAgCQlJ -V0xfRVJSKHByaXYsICJyeCB3cm9uZyBzaXplIGRhdGFcbiIpOw0KPiA+PiAtCQlyZXR1cm4gdHJ1 -ZTsNCj4gPj4gKwkJcmV0dXJuIGZhbHNlOw0KPiA+PiAgCX0NCj4gPj4gIAltZW1jcHkocmVzdW1l -X2RhdGEtPmNtZCwgcGt0LT5kYXRhLCBzaXplb2YoKnJlc3VtZV9kYXRhLT5jbWQpKTsNCj4gPj4g -IAlyZXN1bWVfZGF0YS0+dmFsaWQgPSB0cnVlOw0KPiANCg0K +On Wed, 2015-06-10 at 12:58 -0400, Nicholas Krause wrote: +> +> On June 10, 2015 12:50:45 PM EDT, "Grumbach, Emmanuel" <emmanuel.grumbach@intel.com> wrote: +> >On Wed, 2015-06-10 at 12:33 -0400, Nicholas Krause wrote: +> >> This makes the function iwl_resume_status_fn return false now if +> >> the received packet of type iwl_rx_packet is not the same size +> >> as the structure pointer, iwl_resume_data's cmd element in order +> >> to signal callers about this error and allow them to handle it +> >> occurrently. +> >> +> > +> >Hm... Did you actually hit this if? +> >I am not sure I really want to wait here (which is what will happen if +> >you return false) when we get an unexpected length? I do not expect +> >anything besides the response I am waiting for since the firmware is +> >handling the GET_STATUS *only* - it just came back from WoWLAN. Bottom +> >line, this is really an error path and I prefer to exit and not wait +> >for +> >the timeout in that case. +> >But I might be missing something? +> > +> Why not wait for the time out? Seems there is no reason not to and in that case +> if the firmware handles this + +I doubt it will. This goes back to my original question: did you really +hit this path? Does the patch solves a real bug you faced? + +> why is the if statement still here. + +To get a debug print? :) + +> Nick +> >> Signed-off-by: Nicholas Krause <xerofoify@gmail.com> +> >> --- +> >> drivers/net/wireless/iwlwifi/dvm/mac80211.c | 2 +- +> >> 1 file changed, 1 insertion(+), 1 deletion(-) +> >> +> >> diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >b/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >> index 5abd62e..21e808c 100644 +> >> --- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >> +++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c +> >> @@ -409,7 +409,7 @@ static bool iwl_resume_status_fn(struct +> >iwl_notif_wait_data *notif_wait, +> >> +> >> if (iwl_rx_packet_payload_len(pkt) != sizeof(*resume_data->cmd)) { +> >> IWL_ERR(priv, "rx wrong size data\n"); +> >> - return true; +> >> + return false; +> >> } +> >> memcpy(resume_data->cmd, pkt->data, sizeof(*resume_data->cmd)); +> >> resume_data->valid = true; +> diff --git a/a/content_digest b/N2/content_digest index 986d31c..f05bfbf 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -19,44 +19,58 @@ " linux-wireless@vger.kernel.org <linux-wireless@vger.kernel.org>\0" "\00:1\0" "b\0" - "T24gV2VkLCAyMDE1LTA2LTEwIGF0IDEyOjU4IC0wNDAwLCBOaWNob2xhcyBLcmF1c2Ugd3JvdGU6\n" - "DQo+IA0KPiBPbiBKdW5lIDEwLCAyMDE1IDEyOjUwOjQ1IFBNIEVEVCwgIkdydW1iYWNoLCBFbW1h\n" - "bnVlbCIgPGVtbWFudWVsLmdydW1iYWNoQGludGVsLmNvbT4gd3JvdGU6DQo+ID5PbiBXZWQsIDIw\n" - "MTUtMDYtMTAgYXQgMTI6MzMgLTA0MDAsIE5pY2hvbGFzIEtyYXVzZSB3cm90ZToNCj4gPj4gVGhp\n" - "cyBtYWtlcyB0aGUgZnVuY3Rpb24gaXdsX3Jlc3VtZV9zdGF0dXNfZm4gcmV0dXJuIGZhbHNlIG5v\n" - "dyBpZg0KPiA+PiB0aGUgcmVjZWl2ZWQgcGFja2V0IG9mIHR5cGUgaXdsX3J4X3BhY2tldCBpcyBu\n" - "b3QgdGhlIHNhbWUgc2l6ZQ0KPiA+PiBhcyB0aGUgc3RydWN0dXJlIHBvaW50ZXIsIGl3bF9yZXN1\n" - "bWVfZGF0YSdzIGNtZCBlbGVtZW50IGluIG9yZGVyDQo+ID4+IHRvIHNpZ25hbCBjYWxsZXJzIGFi\n" - "b3V0IHRoaXMgZXJyb3IgYW5kIGFsbG93IHRoZW0gdG8gaGFuZGxlIGl0DQo+ID4+IG9jY3VycmVu\n" - "dGx5Lg0KPiA+PiANCj4gPg0KPiA+SG0uLi4gRGlkIHlvdSBhY3R1YWxseSBoaXQgdGhpcyBpZj8N\n" - "Cj4gPkkgYW0gbm90IHN1cmUgSSByZWFsbHkgd2FudCB0byB3YWl0IGhlcmUgKHdoaWNoIGlzIHdo\n" - "YXQgd2lsbCBoYXBwZW4gaWYNCj4gPnlvdSByZXR1cm4gZmFsc2UpIHdoZW4gd2UgZ2V0IGFuIHVu\n" - "ZXhwZWN0ZWQgbGVuZ3RoPyBJIGRvIG5vdCBleHBlY3QNCj4gPmFueXRoaW5nIGJlc2lkZXMgdGhl\n" - "IHJlc3BvbnNlIEkgYW0gd2FpdGluZyBmb3Igc2luY2UgdGhlIGZpcm13YXJlIGlzDQo+ID5oYW5k\n" - "bGluZyB0aGUgR0VUX1NUQVRVUyAqb25seSogLSBpdCBqdXN0IGNhbWUgYmFjayBmcm9tIFdvV0xB\n" - "Ti4gQm90dG9tDQo+ID5saW5lLCB0aGlzIGlzIHJlYWxseSBhbiBlcnJvciBwYXRoIGFuZCBJIHBy\n" - "ZWZlciB0byBleGl0IGFuZCBub3Qgd2FpdA0KPiA+Zm9yDQo+ID50aGUgdGltZW91dCBpbiB0aGF0\n" - "IGNhc2UuDQo+ID5CdXQgSSBtaWdodCBiZSBtaXNzaW5nIHNvbWV0aGluZz8NCj4gPg0KPiBXaHkg\n" - "bm90IHdhaXQgZm9yIHRoZSB0aW1lIG91dD8gIFNlZW1zIHRoZXJlIGlzIG5vIHJlYXNvbiBub3Qg\n" - "dG8gYW5kIGluIHRoYXQgY2FzZQ0KPiBpZiB0aGUgZmlybXdhcmUgaGFuZGxlcyB0aGlzDQoNCkkg\n" - "ZG91YnQgaXQgd2lsbC4gVGhpcyBnb2VzIGJhY2sgdG8gbXkgb3JpZ2luYWwgcXVlc3Rpb246IGRp\n" - "ZCB5b3UgcmVhbGx5DQpoaXQgdGhpcyBwYXRoPyBEb2VzIHRoZSBwYXRjaCBzb2x2ZXMgYSByZWFs\n" - "IGJ1ZyB5b3UgZmFjZWQ/DQoNCj4gd2h5IGlzIHRoZSBpZiBzdGF0ZW1lbnQgc3RpbGwgaGVyZS4g\n" - "DQoNClRvIGdldCBhIGRlYnVnIHByaW50PyA6KQ0KDQo+IE5pY2sgDQo+ID4+IFNpZ25lZC1vZmYt\n" - "Ynk6IE5pY2hvbGFzIEtyYXVzZSA8eGVyb2ZvaWZ5QGdtYWlsLmNvbT4NCj4gPj4gLS0tDQo+ID4+\n" - "ICBkcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jIHwgMiArLQ0KPiA+\n" - "PiAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pDQo+ID4+IA0K\n" - "PiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAy\n" - "MTEuYw0KPiA+Yi9kcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5jDQo+\n" - "ID4+IGluZGV4IDVhYmQ2MmUuLjIxZTgwOGMgMTAwNjQ0DQo+ID4+IC0tLSBhL2RyaXZlcnMvbmV0\n" - "L3dpcmVsZXNzL2l3bHdpZmkvZHZtL21hYzgwMjExLmMNCj4gPj4gKysrIGIvZHJpdmVycy9uZXQv\n" - "d2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAyMTEuYw0KPiA+PiBAQCAtNDA5LDcgKzQwOSw3IEBA\n" - "IHN0YXRpYyBib29sIGl3bF9yZXN1bWVfc3RhdHVzX2ZuKHN0cnVjdA0KPiA+aXdsX25vdGlmX3dh\n" - "aXRfZGF0YSAqbm90aWZfd2FpdCwNCj4gPj4gIA0KPiA+PiAgCWlmIChpd2xfcnhfcGFja2V0X3Bh\n" - "eWxvYWRfbGVuKHBrdCkgIT0gc2l6ZW9mKCpyZXN1bWVfZGF0YS0+Y21kKSkgew0KPiA+PiAgCQlJ\n" - "V0xfRVJSKHByaXYsICJyeCB3cm9uZyBzaXplIGRhdGFcbiIpOw0KPiA+PiAtCQlyZXR1cm4gdHJ1\n" - "ZTsNCj4gPj4gKwkJcmV0dXJuIGZhbHNlOw0KPiA+PiAgCX0NCj4gPj4gIAltZW1jcHkocmVzdW1l\n" - "X2RhdGEtPmNtZCwgcGt0LT5kYXRhLCBzaXplb2YoKnJlc3VtZV9kYXRhLT5jbWQpKTsNCj4gPj4g\n" - IAlyZXN1bWVfZGF0YS0+dmFsaWQgPSB0cnVlOw0KPiANCg0K + "On Wed, 2015-06-10 at 12:58 -0400, Nicholas Krause wrote:\n" + "> \n" + "> On June 10, 2015 12:50:45 PM EDT, \"Grumbach, Emmanuel\" <emmanuel.grumbach@intel.com> wrote:\n" + "> >On Wed, 2015-06-10 at 12:33 -0400, Nicholas Krause wrote:\n" + "> >> This makes the function iwl_resume_status_fn return false now if\n" + "> >> the received packet of type iwl_rx_packet is not the same size\n" + "> >> as the structure pointer, iwl_resume_data's cmd element in order\n" + "> >> to signal callers about this error and allow them to handle it\n" + "> >> occurrently.\n" + "> >> \n" + "> >\n" + "> >Hm... Did you actually hit this if?\n" + "> >I am not sure I really want to wait here (which is what will happen if\n" + "> >you return false) when we get an unexpected length? I do not expect\n" + "> >anything besides the response I am waiting for since the firmware is\n" + "> >handling the GET_STATUS *only* - it just came back from WoWLAN. Bottom\n" + "> >line, this is really an error path and I prefer to exit and not wait\n" + "> >for\n" + "> >the timeout in that case.\n" + "> >But I might be missing something?\n" + "> >\n" + "> Why not wait for the time out? Seems there is no reason not to and in that case\n" + "> if the firmware handles this\n" + "\n" + "I doubt it will. This goes back to my original question: did you really\n" + "hit this path? Does the patch solves a real bug you faced?\n" + "\n" + "> why is the if statement still here. \n" + "\n" + "To get a debug print? :)\n" + "\n" + "> Nick \n" + "> >> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>\n" + "> >> ---\n" + "> >> drivers/net/wireless/iwlwifi/dvm/mac80211.c | 2 +-\n" + "> >> 1 file changed, 1 insertion(+), 1 deletion(-)\n" + "> >> \n" + "> >> diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >b/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >> index 5abd62e..21e808c 100644\n" + "> >> --- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >> +++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c\n" + "> >> @@ -409,7 +409,7 @@ static bool iwl_resume_status_fn(struct\n" + "> >iwl_notif_wait_data *notif_wait,\n" + "> >> \n" + "> >> \tif (iwl_rx_packet_payload_len(pkt) != sizeof(*resume_data->cmd)) {\n" + "> >> \t\tIWL_ERR(priv, \"rx wrong size data\\n\");\n" + "> >> -\t\treturn true;\n" + "> >> +\t\treturn false;\n" + "> >> \t}\n" + "> >> \tmemcpy(resume_data->cmd, pkt->data, sizeof(*resume_data->cmd));\n" + "> >> \tresume_data->valid = true;\n" + > -0ca522a11c8d985142d94dc8dda2668bce5860ce71d545ddf3f9d8b499045ed1 +23643e5bbada3ca972e2d8eda6503e8a85a54e662c4c0fbb60e72232e7b4fdf0
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.