Fix the warning reported by cocci check.
Changes:
In queue_work fw dereference before it actually get assigned.
move queue_work before gb_bootrom_set_timeout.
As gb_bootrom_get_firmware () return NEXT_REQ_READY_TO_BOOT
only when there is no error and offset + size is actually equal
to fw->size. So initialized next_request to NEXT_REQ_GET_FIRMWARE
for return in other case.
Signed-off-by: Saurav Girepunje <[email protected]>
---
drivers/staging/greybus/bootrom.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
index a8efb86..f54514e 100644
--- a/drivers/staging/greybus/bootrom.c
+++ b/drivers/staging/greybus/bootrom.c
@@ -246,7 +246,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
struct gb_bootrom_get_firmware_response *firmware_response;
struct device *dev = &op->connection->bundle->dev;
unsigned int offset, size;
- enum next_request_type next_request;
+ enum next_request_type next_request = NEXT_REQ_GET_FIRMWARE;
int ret = 0;
/* Disable timeouts */
@@ -296,13 +296,11 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
unlock:
mutex_unlock(&bootrom->mutex);
-queue_work:
/* Refresh timeout */
if (!ret && (offset + size == fw->size))
next_request = NEXT_REQ_READY_TO_BOOT;
- else
- next_request = NEXT_REQ_GET_FIRMWARE;
+queue_work:
gb_bootrom_set_timeout(bootrom, next_request, NEXT_REQ_TIMEOUT_MS);
return ret;
--
1.9.1
On Sun, Jan 26, 2020 at 02:01:30PM +0530, Saurav Girepunje wrote:
> Fix the warning reported by cocci check.
>
> Changes:
>
> In queue_work fw dereference before it actually get assigned.
> move queue_work before gb_bootrom_set_timeout.
Nope. As I said yesterday, you need to verify the output of any static
checkers you use.
The code may be unnecessarily subtle, but there's no way fw can be
dereferenced before being initialised currently.
> -queue_work:
> /* Refresh timeout */
> if (!ret && (offset + size == fw->size))
Specifically, the second operand is never evaluated if ret is non-zero.
> next_request = NEXT_REQ_READY_TO_BOOT;
> - else
> - next_request = NEXT_REQ_GET_FIRMWARE;
>
> +queue_work:
> gb_bootrom_set_timeout(bootrom, next_request, NEXT_REQ_TIMEOUT_MS);
>
> return ret;
Johan
On Sun, Jan 26, 2020 at 02:01:30PM +0530, Saurav Girepunje wrote:
> Fix the warning reported by cocci check.
What is "cocci check"?
> Changes:
>
Why add that line?
> In queue_work fw dereference before it actually get assigned.
> move queue_work before gb_bootrom_set_timeout.
>
> As gb_bootrom_get_firmware () return NEXT_REQ_READY_TO_BOOT
> only when there is no error and offset + size is actually equal
> to fw->size. So initialized next_request to NEXT_REQ_GET_FIRMWARE
> for return in other case.
>
> Signed-off-by: Saurav Girepunje <[email protected]>
> ---
> drivers/staging/greybus/bootrom.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
As Johan said, there are a lot of really bad "static checking"
tools out there that can not properly parse C code. Always verify by
hand what the tools said is wrong, really is an issue before sending a
patch out for something that is not correct. This looks like you need
to use a better tool.
greg k-h