Skip to contentSkip to conversation

i2c-scmi: remove some bogus NULL checks

PATCH Applied Dan Carpenter 2 participants 4 messages Oct 19, 2013 – Oct 31, 2013 1 message not in the archive

Subsystems (2)
Kernel janitors I2C subsystem host drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 i2c-scmi: remove some bogus NULL checks

"obj" can't be NULL here.

We already know that "pkg->package.elements" gives us a valid pointer so the next pointer after that is also non-NULL.

Signed-off-by Dan Carpenter <[email protected]>
drivers/i2c/busses/i2c-scmi.c +3 -3
index c447e8d..5992355 100644
@@ -223,7 +223,7 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 		goto out;
 
 	obj = pkg->package.elements + 1;
-	if (obj = NULL || obj->type != ACPI_TYPE_INTEGER) {
+	if (obj->type != ACPI_TYPE_INTEGER) {
 		ACPI_ERROR((AE_INFO, "Invalid argument type"));
 		result = -EIO;
 		goto out;

@@ -235,7 +235,7 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 	case I2C_SMBUS_BYTE:
 	case I2C_SMBUS_BYTE_DATA:
 	case I2C_SMBUS_WORD_DATA:
-		if (obj = NULL || obj->type != ACPI_TYPE_INTEGER) {
+		if (obj->type != ACPI_TYPE_INTEGER) {
 			ACPI_ERROR((AE_INFO, "Invalid argument type"));
 			result = -EIO;
 			goto out;

@@ -246,7 +246,7 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 			data->byte = obj->integer.value;
 		break;
 	case I2C_SMBUS_BLOCK_DATA:
-		if (obj = NULL || obj->type != ACPI_TYPE_BUFFER) {
+		if (obj->type != ACPI_TYPE_BUFFER) {
 			ACPI_ERROR((AE_INFO, "Invalid argument type"));
 			result = -EIO;
 			goto out;

A message that is not in the archive (sent off-list or not yet received): 20131019084622.GA9312-dZEljifmRObu9KfB+GxooP8+0UxHXcjY@public.gmane.org

#2 lore raw
Quoted 4 lines · Dan Carpenter

"obj" can't be NULL here.

We already know that "pkg->package.elements" gives us a valid pointer so the next pointer after that is also non-NULL.

Why is that? Can't see it...

Quoted 34 lines
Signed-off-by Dan Carpenter <[email protected]>
drivers/i2c/busses/i2c-scmi.c +3 -3
index c447e8d..5992355 100644
@@ -223,7 +223,7 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 		goto out;
 
 	obj = pkg->package.elements + 1;
-	if (obj == NULL || obj->type != ACPI_TYPE_INTEGER) {
+	if (obj->type != ACPI_TYPE_INTEGER) {
 		ACPI_ERROR((AE_INFO, "Invalid argument type"));
 		result = -EIO;
 		goto out;

@@ -235,7 +235,7 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 	case I2C_SMBUS_BYTE:
 	case I2C_SMBUS_BYTE_DATA:
 	case I2C_SMBUS_WORD_DATA:
-		if (obj == NULL || obj->type != ACPI_TYPE_INTEGER) {
+		if (obj->type != ACPI_TYPE_INTEGER) {
 			ACPI_ERROR((AE_INFO, "Invalid argument type"));
 			result = -EIO;
 			goto out;

@@ -246,7 +246,7 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 			data->byte = obj->integer.value;
 		break;
 	case I2C_SMBUS_BLOCK_DATA:
-		if (obj == NULL || obj->type != ACPI_TYPE_BUFFER) {
+		if (obj->type != ACPI_TYPE_BUFFER) {
 			ACPI_ERROR((AE_INFO, "Invalid argument type"));
 			result = -EIO;
 			goto out;
author
#3 lore raw
Quoted 11 lines · Wolfram Sang

drivers/i2c/busses/i2c-scmi.c acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags, · line 229

Quoted 11 lines
drivers/i2c/busses/i2c-scmi.c +1 -1
@@ -223,7 +223,7 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 		goto out;
 
 	obj = pkg->package.elements + 1;
-	if (obj = NULL || obj->type != ACPI_TYPE_INTEGER) {
+	if (obj->type != ACPI_TYPE_INTEGER) {
 		ACPI_ERROR((AE_INFO, "Invalid argument type"));
 		result = -EIO;
 		goto out;

For obj to be 0 then pkg->package.elements must be -1. If we're getting a pointer of -1 then we're already screwed. Also we already used pkg->elements without the "+ 1" so we know that's not -1.

The reason for this patch was that some compilers assume pointer math can't overflow and my checker was complaining. We use -fno-strict-overflow in the kernel, but it still seemed worth cleaning.

regards,
dan carpenter