mtd: spi-nor: Fix an error code in spi_nor_read_raw()
Subsystems (3)
PATCH 1/1 mtd: spi-nor: Fix an error code in spi_nor_read_raw()
The problem is that if "ret" is negative then when we check if "ret > len", that condition is going to be true because of type promotion. So this patch re-orders the code to check for negatives first and preserve those error codes.
--- drivers/mtd/spi-nor/spi-nor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +2 -2
@@ -2903,10 +2903,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)while (len) { ret =spi_nor_read_data (nor, addr, len, buf); -if (!ret || ret > len) -return -EIO ;if (ret <0 )return ret; +if (!ret || ret > len) +return -EIO ; buf += ret; addr += ret;
Signature · 1 line
2.20.1
Am 15.08.2019 10:32, schrieb Dan Carpenter:
Quoted 16 lines
The problem is that if "ret" is negative then when we check if "ret > len", that condition is going to be true because of type promotion. So this patch re-orders the code to check for negatives first and preserve those error codes.
Fixes f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables")Signed-off-by Dan Carpenter <[email protected]>--- drivers/mtd/spi-nor/spi-nor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/mtd/spi-nor/spi-nor.c +0
-0
drivers/mtd/spi-nor/spi-nor.c static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf) · line 2909
Quoted 9 lines
drivers/mtd/spi-nor/spi-nor.c +2
-2@@ -2903,10 +2903,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)while (len) { ret =spi_nor_read_data (nor, addr, len, buf); -if (!ret || ret > len) -return -EIO ;if (ret <0 )return ret; +if (!ret || ret > len) +return -EIO ;
Bonuspoints to make this more readable:
if (ret=0 || ret > len) return -EIO;
that makes the intention more obvious.
JM2C,
wh
Quoted 3 lines
drivers/mtd/spi-nor/spi-nor.c +0
-0buf += ret; addr += ret;
Quoted 31 lines · walter harms
Am 15.08.2019 10:32, schrieb Dan Carpenter:
Quoted 16 lines
The problem is that if "ret" is negative then when we check if "ret > len", that condition is going to be true because of type promotion. So this patch re-orders the code to check for negatives first and preserve those error codes.
Fixes f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables")Signed-off-by Dan Carpenter <[email protected]>--- drivers/mtd/spi-nor/spi-nor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/mtd/spi-nor/spi-nor.c +0
-0drivers/mtd/spi-nor/spi-nor.c static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf) · line 2909
Quoted 9 lines
drivers/mtd/spi-nor/spi-nor.c +2
-2@@ -2903,10 +2903,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf)while (len) { ret =spi_nor_read_data (nor, addr, len, buf); -if (!ret || ret > len) -return -EIO ;if (ret <0 )return ret; +if (!ret || ret > len) +return -EIO ;Bonuspoints to make this more readable:
Quoted 4 lines
if (ret=0 || ret > len) return -EIO;that makes the intention more obvious.
That's not really related to the bug fix.
I do agree with your style though. "ret" is a number here, and not a bool or error vs no error so = 0 is nicer.
regards,
dan carpenter
DQoNCk9uIDA4LzE1LzIwMTkgMTE6MzIgQU0sIERhbiBDYXJwZW50ZXIgd3JvdGU6DQo+IEV4dGVy bmFsIEUtTWFpbA0KPiANCj4gDQo+IFRoZSBwcm9ibGVtIGlzIHRoYXQgaWYgInJldCIgaXMgbmVn YXRpdmUgdGhlbiB3aGVuIHdlIGNoZWNrIGlmDQo+ICJyZXQgPiBsZW4iLCB0aGF0IGNvbmRpdGlv biBpcyBnb2luZyB0byBiZSB0cnVlIGJlY2F1c2Ugb2YgdHlwZQ0KPiBwcm9tb3Rpb24uICBTbyB0 aGlzIHBhdGNoIHJlLW9yZGVycyB0aGUgY29kZSB0byBjaGVjayBmb3IgbmVnYXRpdmVzDQo+IGZp cnN0IGFuZCBwcmVzZXJ2ZSB0aG9zZSBlcnJvciBjb2Rlcy4NCj4gDQo+IEZpeGVzOiBmMzg0YjM1 MmNiZjAgKCJtdGQ6IHNwaS1ub3I6IHBhcnNlIFNlcmlhbCBGbGFzaCBEaXNjb3ZlcmFibGUgUGFy YW1ldGVycyAoU0ZEUCkgdGFibGVzIikNCj4gU2lnbmVkLW9mZi1ieTogRGFuIENhcnBlbnRlciA8 ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29tPg0KDQpSZXZpZXdlZC1ieTogVHVkb3IgQW1iYXJ1cyA8 dHVkb3IuYW1iYXJ1c0BtaWNyb2NoaXAuY29tPg0KDQo+IC0tLQ0KPiAgZHJpdmVycy9tdGQvc3Bp LW5vci9zcGktbm9yLmMgfCA0ICsrLS0NCj4gIDEgZmlsZSBjaGFuZ2VkLCAyIGluc2VydGlvbnMo KyksIDIgZGVsZXRpb25zKC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9tdGQvc3BpLW5v ci9zcGktbm9yLmMgYi9kcml2ZXJzL210ZC9zcGktbm9yL3NwaS1ub3IuYw0KPiBpbmRleCA2M2Fm ODc2MDliYWMuLjk4NmIwNzU0NDk1ZCAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9tdGQvc3BpLW5v ci9zcGktbm9yLmMNCj4gKysrIGIvZHJpdmVycy9tdGQvc3BpLW5vci9zcGktbm9yLmMNCj4gQEAg LTI5MDMsMTAgKzI5MDMsMTAgQEAgc3RhdGljIGludCBzcGlfbm9yX3JlYWRfcmF3KHN0cnVjdCBz cGlfbm9yICpub3IsIHUzMiBhZGRyLCBzaXplX3QgbGVuLCB1OCAqYnVmKQ0KPiAgDQo+ICAJd2hp bGUgKGxlbikgew0KPiAgCQlyZXQgPSBzcGlfbm9yX3JlYWRfZGF0YShub3IsIGFkZHIsIGxlbiwg YnVmKTsNCj4gLQkJaWYgKCFyZXQgfHwgcmV0ID4gbGVuKQ0KPiAtCQkJcmV0dXJuIC1FSU87DQo+ ICAJCWlmIChyZXQgPCAwKQ0KPiAgCQkJcmV0dXJuIHJldDsNCj4gKwkJaWYgKCFyZXQgfHwgcmV0 ID4gbGVuKQ0KPiArCQkJcmV0dXJuIC1FSU87DQo+ICANCj4gIAkJYnVmICs9IHJldDsNCj4gIAkJ YWRkciArPSByZXQ7DQo+IA0K
DQoNCk9uIDA4LzE1LzIwMTkgMTE6MzIgQU0sIERhbiBDYXJwZW50ZXIgd3JvdGU6DQo+IEV4dGVy bmFsIEUtTWFpbA0KPiANCj4gDQo+IFRoZSBwcm9ibGVtIGlzIHRoYXQgaWYgInJldCIgaXMgbmVn YXRpdmUgdGhlbiB3aGVuIHdlIGNoZWNrIGlmDQo+ICJyZXQgPiBsZW4iLCB0aGF0IGNvbmRpdGlv biBpcyBnb2luZyB0byBiZSB0cnVlIGJlY2F1c2Ugb2YgdHlwZQ0KPiBwcm9tb3Rpb24uICBTbyB0 aGlzIHBhdGNoIHJlLW9yZGVycyB0aGUgY29kZSB0byBjaGVjayBmb3IgbmVnYXRpdmVzDQo+IGZp cnN0IGFuZCBwcmVzZXJ2ZSB0aG9zZSBlcnJvciBjb2Rlcy4NCj4gDQo+IEZpeGVzOiBmMzg0YjM1 MmNiZjAgKCJtdGQ6IHNwaS1ub3I6IHBhcnNlIFNlcmlhbCBGbGFzaCBEaXNjb3ZlcmFibGUgUGFy YW1ldGVycyAoU0ZEUCkgdGFibGVzIikNCj4gU2lnbmVkLW9mZi1ieTogRGFuIENhcnBlbnRlciA8 ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29tPg0KPiAtLS0NCj4gIGRyaXZlcnMvbXRkL3NwaS1ub3Iv c3BpLW5vci5jIHwgNCArKy0tDQo+ICAxIGZpbGUgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCAy IGRlbGV0aW9ucygtKQ0KPiANCg0KU2luY2UgdGhlIGJ1ZyB3YXMgbm90IGludHJvZHVjZWQgaW4g dGhlIHByZXZpb3VzIHJlbGVhc2UgYW5kIHdlIGFyZSBxdWl0ZSBsYXRlDQpmb3IgbXRkL2ZpeGVz LA0KDQpBcHBsaWVkIHRvIGh0dHBzOi8vZ2l0Lmtlcm5lbC5vcmcvcHViL3NjbS9saW51eC9rZXJu ZWwvZ2l0L210ZC9saW51eC5naXQsDQpzcGktbm9yL25leHQgYnJhbmNoLg0KDQpUaGFua3MsDQp0 YQ0K