* [PATCH] staging: greybus: bootrom: fix uninitialized variables
@ 2020-01-25 8:44 ` Saurav Girepunje
0 siblings, 0 replies; 10+ messages in thread
From: Saurav Girepunje @ 2020-01-25 8:44 UTC (permalink / raw)
To: vireshk, johan, elder, gregkh, greybus-dev, devel, linux-kernel
Cc: saurav.girepunje
fix uninitialized variables issue found using static code analysis tool
(error) Uninitialized variable: offset
(error) Uninitialized variable: size
Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
---
drivers/staging/greybus/bootrom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
index a8efb86..9eabeb3 100644
--- a/drivers/staging/greybus/bootrom.c
+++ b/drivers/staging/greybus/bootrom.c
@@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
struct gb_bootrom_get_firmware_request *firmware_request;
struct gb_bootrom_get_firmware_response *firmware_response;
struct device *dev = &op->connection->bundle->dev;
- unsigned int offset, size;
+ unsigned int offset = 0, size = 0;
enum next_request_type next_request;
int ret = 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] staging: greybus: bootrom: fix uninitialized variables
@ 2020-01-25 8:44 ` Saurav Girepunje
0 siblings, 0 replies; 10+ messages in thread
From: Saurav Girepunje @ 2020-01-25 8:44 UTC (permalink / raw)
To: vireshk, johan, elder, gregkh, greybus-dev, devel, linux-kernel
Cc: saurav.girepunje
fix uninitialized variables issue found using static code analysis tool
(error) Uninitialized variable: offset
(error) Uninitialized variable: size
Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
---
drivers/staging/greybus/bootrom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
index a8efb86..9eabeb3 100644
--- a/drivers/staging/greybus/bootrom.c
+++ b/drivers/staging/greybus/bootrom.c
@@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
struct gb_bootrom_get_firmware_request *firmware_request;
struct gb_bootrom_get_firmware_response *firmware_response;
struct device *dev = &op->connection->bundle->dev;
- unsigned int offset, size;
+ unsigned int offset = 0, size = 0;
enum next_request_type next_request;
int ret = 0;
--
1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: greybus: bootrom: fix uninitialized variables
2020-01-25 8:44 ` Saurav Girepunje
@ 2020-01-25 10:00 ` Johan Hovold
-1 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2020-01-25 10:00 UTC (permalink / raw)
To: Saurav Girepunje
Cc: vireshk, johan, elder, gregkh, greybus-dev, devel, linux-kernel,
saurav.girepunje
On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
> fix uninitialized variables issue found using static code analysis tool
Which tool is that?
> (error) Uninitialized variable: offset
> (error) Uninitialized variable: size
>
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> ---
> drivers/staging/greybus/bootrom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
> index a8efb86..9eabeb3 100644
> --- a/drivers/staging/greybus/bootrom.c
> +++ b/drivers/staging/greybus/bootrom.c
> @@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
> struct gb_bootrom_get_firmware_request *firmware_request;
> struct gb_bootrom_get_firmware_response *firmware_response;
> struct device *dev = &op->connection->bundle->dev;
> - unsigned int offset, size;
> + unsigned int offset = 0, size = 0;
> enum next_request_type next_request;
> int ret = 0;
I think this has come up in the past, and while the code in question is
overly complicated and confuses static checkers as well as humans, it
looks correct to me.
Please make sure to verify the output of any tools before posting
patches based on them.
Johan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: greybus: bootrom: fix uninitialized variables
@ 2020-01-25 10:00 ` Johan Hovold
0 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2020-01-25 10:00 UTC (permalink / raw)
To: Saurav Girepunje
Cc: devel, elder, vireshk, johan, linux-kernel, greybus-dev,
saurav.girepunje, gregkh
On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
> fix uninitialized variables issue found using static code analysis tool
Which tool is that?
> (error) Uninitialized variable: offset
> (error) Uninitialized variable: size
>
> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
> ---
> drivers/staging/greybus/bootrom.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
> index a8efb86..9eabeb3 100644
> --- a/drivers/staging/greybus/bootrom.c
> +++ b/drivers/staging/greybus/bootrom.c
> @@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
> struct gb_bootrom_get_firmware_request *firmware_request;
> struct gb_bootrom_get_firmware_response *firmware_response;
> struct device *dev = &op->connection->bundle->dev;
> - unsigned int offset, size;
> + unsigned int offset = 0, size = 0;
> enum next_request_type next_request;
> int ret = 0;
I think this has come up in the past, and while the code in question is
overly complicated and confuses static checkers as well as humans, it
looks correct to me.
Please make sure to verify the output of any tools before posting
patches based on them.
Johan
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: greybus: bootrom: fix uninitialized variables
2020-01-25 10:00 ` Johan Hovold
@ 2020-01-25 12:14 ` SAURAV GIREPUNJE
-1 siblings, 0 replies; 10+ messages in thread
From: SAURAV GIREPUNJE @ 2020-01-25 12:14 UTC (permalink / raw)
To: Johan Hovold; +Cc: vireshk, elder, gregkh, greybus-dev, devel, linux-kernel
On 25/01/20 11:00 +0100, Johan Hovold wrote:
>On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
>> fix uninitialized variables issue found using static code analysis tool
>
>Which tool is that?
>
>> (error) Uninitialized variable: offset
>> (error) Uninitialized variable: size
>>
>> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
>> ---
>> drivers/staging/greybus/bootrom.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
>> index a8efb86..9eabeb3 100644
>> --- a/drivers/staging/greybus/bootrom.c
>> +++ b/drivers/staging/greybus/bootrom.c
>> @@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
>> struct gb_bootrom_get_firmware_request *firmware_request;
>> struct gb_bootrom_get_firmware_response *firmware_response;
>> struct device *dev = &op->connection->bundle->dev;
>> - unsigned int offset, size;
>> + unsigned int offset = 0, size = 0;
>> enum next_request_type next_request;
>> int ret = 0;
>
>I think this has come up in the past, and while the code in question is
>overly complicated and confuses static checkers as well as humans, it
>looks correct to me.
>
>Please make sure to verify the output of any tools before posting
>patches based on them.
>
>Johan
I used cppcheck tool .
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: greybus: bootrom: fix uninitialized variables
@ 2020-01-25 12:14 ` SAURAV GIREPUNJE
0 siblings, 0 replies; 10+ messages in thread
From: SAURAV GIREPUNJE @ 2020-01-25 12:14 UTC (permalink / raw)
To: Johan Hovold; +Cc: devel, elder, vireshk, linux-kernel, greybus-dev, gregkh
On 25/01/20 11:00 +0100, Johan Hovold wrote:
>On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
>> fix uninitialized variables issue found using static code analysis tool
>
>Which tool is that?
>
>> (error) Uninitialized variable: offset
>> (error) Uninitialized variable: size
>>
>> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
>> ---
>> drivers/staging/greybus/bootrom.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
>> index a8efb86..9eabeb3 100644
>> --- a/drivers/staging/greybus/bootrom.c
>> +++ b/drivers/staging/greybus/bootrom.c
>> @@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
>> struct gb_bootrom_get_firmware_request *firmware_request;
>> struct gb_bootrom_get_firmware_response *firmware_response;
>> struct device *dev = &op->connection->bundle->dev;
>> - unsigned int offset, size;
>> + unsigned int offset = 0, size = 0;
>> enum next_request_type next_request;
>> int ret = 0;
>
>I think this has come up in the past, and while the code in question is
>overly complicated and confuses static checkers as well as humans, it
>looks correct to me.
>
>Please make sure to verify the output of any tools before posting
>patches based on them.
>
>Johan
I used cppcheck tool .
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: greybus: bootrom: fix uninitialized variables
2020-01-25 8:44 ` Saurav Girepunje
@ 2020-01-27 8:09 ` Dan Carpenter
-1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-01-27 8:09 UTC (permalink / raw)
To: Saurav Girepunje
Cc: vireshk, johan, elder, gregkh, greybus-dev, devel, linux-kernel,
saurav.girepunje
On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
> fix uninitialized variables issue found using static code analysis tool
>
> (error) Uninitialized variable: offset
> (error) Uninitialized variable: size
>
These are false positives as Johan said. Don't change the code just to
make the static analysis tool happy, fix the tools instead.
Also the patch doesn't apply. Read the first paragraph of
Documentation/process/email-clients.rst and figure out why it's not
working.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] staging: greybus: bootrom: fix uninitialized variables
@ 2020-01-27 8:09 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2020-01-27 8:09 UTC (permalink / raw)
To: Saurav Girepunje
Cc: devel, elder, vireshk, johan, linux-kernel, greybus-dev,
saurav.girepunje, gregkh
On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
> fix uninitialized variables issue found using static code analysis tool
>
> (error) Uninitialized variable: offset
> (error) Uninitialized variable: size
>
These are false positives as Johan said. Don't change the code just to
make the static analysis tool happy, fix the tools instead.
Also the patch doesn't apply. Read the first paragraph of
Documentation/process/email-clients.rst and figure out why it's not
working.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [greybus-dev] [PATCH] staging: greybus: bootrom: fix uninitialized variables
2020-01-25 12:14 ` SAURAV GIREPUNJE
@ 2020-01-27 14:10 ` Alex Elder
-1 siblings, 0 replies; 10+ messages in thread
From: Alex Elder @ 2020-01-27 14:10 UTC (permalink / raw)
To: SAURAV GIREPUNJE, Johan Hovold
Cc: devel, elder, vireshk, linux-kernel, greybus-dev
On 1/25/20 6:14 AM, SAURAV GIREPUNJE wrote:
> On 25/01/20 11:00 +0100, Johan Hovold wrote:
>> On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
>>> fix uninitialized variables issue found using static code analysis tool
>>
>> Which tool is that?
>>
>>> (error) Uninitialized variable: offset
>>> (error) Uninitialized variable: size
>>>
>>> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
>>> ---
>>> drivers/staging/greybus/bootrom.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
>>> index a8efb86..9eabeb3 100644
>>> --- a/drivers/staging/greybus/bootrom.c
>>> +++ b/drivers/staging/greybus/bootrom.c
>>> @@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
>>> struct gb_bootrom_get_firmware_request *firmware_request;
>>> struct gb_bootrom_get_firmware_response *firmware_response;
>>> struct device *dev = &op->connection->bundle->dev;
>>> - unsigned int offset, size;
>>> + unsigned int offset = 0, size = 0;
>>> enum next_request_type next_request;
>>> int ret = 0;
>>
>> I think this has come up in the past, and while the code in question is
>> overly complicated and confuses static checkers as well as humans, it
>> looks correct to me.
>>
>> Please make sure to verify the output of any tools before posting
>> patches based on them.
>>
>> Johan
> I used cppcheck tool .
Implied in Johan's question is a suggestion.
When you propose a patch that addresses something flagged by a
tool of some kind, it is good practice to identify the tool in
the patch description, and even better, give an example of how
the tool was invoked when reported the problem you're fixing.
Sometimes people even include the output of the tool, though
I think that can sometimes be a bit much.
And as you have now heard several times, do not blindly trust
the output of these tools. They're intended to call attention
to things for you to examine; they are no match for a human,
and things they tell you about are not guaranteed to be real
problems.
-Alex
> _______________________________________________
> greybus-dev mailing list
> greybus-dev@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/greybus-dev
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [greybus-dev] [PATCH] staging: greybus: bootrom: fix uninitialized variables
@ 2020-01-27 14:10 ` Alex Elder
0 siblings, 0 replies; 10+ messages in thread
From: Alex Elder @ 2020-01-27 14:10 UTC (permalink / raw)
To: SAURAV GIREPUNJE, Johan Hovold
Cc: devel, vireshk, elder, linux-kernel, greybus-dev
On 1/25/20 6:14 AM, SAURAV GIREPUNJE wrote:
> On 25/01/20 11:00 +0100, Johan Hovold wrote:
>> On Sat, Jan 25, 2020 at 02:14:03PM +0530, Saurav Girepunje wrote:
>>> fix uninitialized variables issue found using static code analysis tool
>>
>> Which tool is that?
>>
>>> (error) Uninitialized variable: offset
>>> (error) Uninitialized variable: size
>>>
>>> Signed-off-by: Saurav Girepunje <saurav.girepunje@gmail.com>
>>> ---
>>> drivers/staging/greybus/bootrom.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
>>> index a8efb86..9eabeb3 100644
>>> --- a/drivers/staging/greybus/bootrom.c
>>> +++ b/drivers/staging/greybus/bootrom.c
>>> @@ -245,7 +245,7 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
>>> struct gb_bootrom_get_firmware_request *firmware_request;
>>> struct gb_bootrom_get_firmware_response *firmware_response;
>>> struct device *dev = &op->connection->bundle->dev;
>>> - unsigned int offset, size;
>>> + unsigned int offset = 0, size = 0;
>>> enum next_request_type next_request;
>>> int ret = 0;
>>
>> I think this has come up in the past, and while the code in question is
>> overly complicated and confuses static checkers as well as humans, it
>> looks correct to me.
>>
>> Please make sure to verify the output of any tools before posting
>> patches based on them.
>>
>> Johan
> I used cppcheck tool .
Implied in Johan's question is a suggestion.
When you propose a patch that addresses something flagged by a
tool of some kind, it is good practice to identify the tool in
the patch description, and even better, give an example of how
the tool was invoked when reported the problem you're fixing.
Sometimes people even include the output of the tool, though
I think that can sometimes be a bit much.
And as you have now heard several times, do not blindly trust
the output of these tools. They're intended to call attention
to things for you to examine; they are no match for a human,
and things they tell you about are not guaranteed to be real
problems.
-Alex
> _______________________________________________
> greybus-dev mailing list
> greybus-dev@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/greybus-dev
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-01-27 14:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-25 8:44 [PATCH] staging: greybus: bootrom: fix uninitialized variables Saurav Girepunje
2020-01-25 8:44 ` Saurav Girepunje
2020-01-25 10:00 ` Johan Hovold
2020-01-25 10:00 ` Johan Hovold
2020-01-25 12:14 ` SAURAV GIREPUNJE
2020-01-25 12:14 ` SAURAV GIREPUNJE
2020-01-27 14:10 ` [greybus-dev] " Alex Elder
2020-01-27 14:10 ` Alex Elder
2020-01-27 8:09 ` Dan Carpenter
2020-01-27 8:09 ` Dan Carpenter
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.