[PATCH v1 0/1] Fix uninitialized variable in ipmb_dev_int.c

[PATCH v1 0/1] Fix uninitialized variable in ipmb_dev_int.c

[Asmaa Mnebhi]

ret at line 112 of ipmb_dev_int.c is uninitialized which
results in a warning during build regressions.

Asmaa Mnebhi (1):
  Fix uninitialized variable in ipmb_dev_int.c

 drivers/char/ipmi/ipmb_dev_int.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.1.2

[PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

[Asmaa Mnebhi]

Signed-off-by: Asmaa Mnebhi <Asmaa@mellanox.com>
---
 drivers/char/ipmi/ipmb_dev_int.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
index 5720433..285e0b8 100644
--- a/drivers/char/ipmi/ipmb_dev_int.c
+++ b/drivers/char/ipmi/ipmb_dev_int.c
@@ -76,7 +76,7 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
 	struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
 	struct ipmb_request_elem *queue_elem;
 	struct ipmb_msg msg;
-	ssize_t ret;
+	ssize_t ret = 0;
 
 	memset(&msg, 0, sizeof(msg));
 
-- 
2.1.2

Re: [PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

[Corey Minyard]

On Wed, Jul 24, 2019 at 10:36:42AM -0400, Asmaa Mnebhi wrote:
> Signed-off-by: Asmaa Mnebhi <Asmaa@mellanox.com>

The patch is, of course, fine, but you should add some info
about how it was found and a Reported-by: tag.

Thanks,

-corey

> ---
>  drivers/char/ipmi/ipmb_dev_int.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
> index 5720433..285e0b8 100644
> --- a/drivers/char/ipmi/ipmb_dev_int.c
> +++ b/drivers/char/ipmi/ipmb_dev_int.c
> @@ -76,7 +76,7 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
>  	struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
>  	struct ipmb_request_elem *queue_elem;
>  	struct ipmb_msg msg;
> -	ssize_t ret;
> +	ssize_t ret = 0;
>  
>  	memset(&msg, 0, sizeof(msg));
>  
> -- 
> 2.1.2
> 

[PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

[Asmaa Mnebhi]

Signed-off-by: Asmaa Mnebhi <Asmaa@mellanox.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/char/ipmi/ipmb_dev_int.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
index 5720433..285e0b8 100644
--- a/drivers/char/ipmi/ipmb_dev_int.c
+++ b/drivers/char/ipmi/ipmb_dev_int.c
@@ -76,7 +76,7 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
 	struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
 	struct ipmb_request_elem *queue_elem;
 	struct ipmb_msg msg;
-	ssize_t ret;
+	ssize_t ret = 0;
 
 	memset(&msg, 0, sizeof(msg));
 
-- 
2.1.2

Re: [PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

[Corey Minyard]

On Wed, Jul 24, 2019 at 01:45:57PM -0400, Asmaa Mnebhi wrote:
> Signed-off-by: Asmaa Mnebhi <Asmaa@mellanox.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>

Sorry to be picky here, but it's considered bad style to have an
empty message.  I probably wasn't clear before, but you should
add some text like "Found by build regression/improvement testing."
or something like that.  Just so people know where it was found.

Could you also add a "Fixes" field?  This is important in case
someone pulls the original patch, they can look forward and see
if any bugs were fixed.  From the kernel docs:

If your patch fixes a bug in a specific commit, e.g. you found an issue using
``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
the SHA-1 ID, and the one line summary.  Do not split the tag across multiple
lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify
parsing scripts.  For example::

        Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")

I was going to do that myself, but since another spin is required...

Thanks,

-corey

> ---
>  drivers/char/ipmi/ipmb_dev_int.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
> index 5720433..285e0b8 100644
> --- a/drivers/char/ipmi/ipmb_dev_int.c
> +++ b/drivers/char/ipmi/ipmb_dev_int.c
> @@ -76,7 +76,7 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
>  	struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
>  	struct ipmb_request_elem *queue_elem;
>  	struct ipmb_msg msg;
> -	ssize_t ret;
> +	ssize_t ret = 0;
>  
>  	memset(&msg, 0, sizeof(msg));
>  
> -- 
> 2.1.2
> 

RE: [PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

[Asmaa Mnebhi]

Ah ok! I put that message on the cover letter. Thanks! I will make an update shortly.

-----Original Message-----
From: Corey Minyard <tcminyard@gmail.com> On Behalf Of Corey Minyard
Sent: Wednesday, July 24, 2019 3:07 PM
To: Asmaa Mnebhi <Asmaa@mellanox.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

On Wed, Jul 24, 2019 at 01:45:57PM -0400, Asmaa Mnebhi wrote:
> Signed-off-by: Asmaa Mnebhi <Asmaa@mellanox.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>

Sorry to be picky here, but it's considered bad style to have an empty message.  I probably wasn't clear before, but you should add some text like "Found by build regression/improvement testing."
or something like that.  Just so people know where it was found.

Could you also add a "Fixes" field?  This is important in case someone pulls the original patch, they can look forward and see if any bugs were fixed.  From the kernel docs:

If your patch fixes a bug in a specific commit, e.g. you found an issue using ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of the SHA-1 ID, and the one line summary.  Do not split the tag across multiple lines, tags are exempt from the "wrap at 75 columns" rule in order to simplify parsing scripts.  For example::

        Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed")

I was going to do that myself, but since another spin is required...

Thanks,

-corey

> ---
>  drivers/char/ipmi/ipmb_dev_int.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmb_dev_int.c 
> b/drivers/char/ipmi/ipmb_dev_int.c
> index 5720433..285e0b8 100644
> --- a/drivers/char/ipmi/ipmb_dev_int.c
> +++ b/drivers/char/ipmi/ipmb_dev_int.c
> @@ -76,7 +76,7 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
>  	struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
>  	struct ipmb_request_elem *queue_elem;
>  	struct ipmb_msg msg;
> -	ssize_t ret;
> +	ssize_t ret = 0;
>  
>  	memset(&msg, 0, sizeof(msg));
>  
> --
> 2.1.2
> 

[PATCH v1 0/1] Fix uninitialized variable in ipmb_dev_int.c

[Asmaa Mnebhi]

ret at line 112 of ipmb_dev_int.c is uninitialized which
results in a warning during build regressions.

This warning was reported during regression testing led by Geert
Uytterhoeven for v5.3-rc1.

Asmaa Mnebhi (1):
  Fix uninitialized variable in ipmb_dev_int.c

 drivers/char/ipmi/ipmb_dev_int.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.1.2

[PATCH v1 0/1] Fix uninitialized variable in ipmb_dev_int.c

[Asmaa Mnebhi]

ret at line 112 of ipmb_dev_int.c is uninitialized which
results in a warning during build regressions.

This warning was reported during regression/improvement
testing led by Geert Uytterhoeven for v5.3-rc1.

Asmaa Mnebhi (1):
  Fix uninitialized variable in ipmb_dev_int.c

 drivers/char/ipmi/ipmb_dev_int.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.1.2

[PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

[Asmaa Mnebhi]

ret at line 112 of ipmb_dev_int.c is uninitialized which
results in a warning during build regressions.
This warning was found by build regression/improvement
testing for v5.3-rc1.

Reported-by: build regression/improvement testing for v5.3-rc1.
Fixes: 51bd6f291583 ("Add support for IPMB driver")
Signed-off-by: Asmaa Mnebhi <Asmaa@mellanox.com>
---
 drivers/char/ipmi/ipmb_dev_int.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
index 5720433..285e0b8 100644
--- a/drivers/char/ipmi/ipmb_dev_int.c
+++ b/drivers/char/ipmi/ipmb_dev_int.c
@@ -76,7 +76,7 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
 	struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
 	struct ipmb_request_elem *queue_elem;
 	struct ipmb_msg msg;
-	ssize_t ret;
+	ssize_t ret = 0;
 
 	memset(&msg, 0, sizeof(msg));
 
-- 
2.1.2

Re: [PATCH v1 1/1] Fix uninitialized variable in ipmb_dev_int.c

[Corey Minyard]

On Wed, Jul 24, 2019 at 03:32:57PM -0400, Asmaa Mnebhi wrote:
> ret at line 112 of ipmb_dev_int.c is uninitialized which
> results in a warning during build regressions.
> This warning was found by build regression/improvement
> testing for v5.3-rc1.

Applied, thanks for sticking with it :).

-corey

> 
> Reported-by: build regression/improvement testing for v5.3-rc1.
> Fixes: 51bd6f291583 ("Add support for IPMB driver")
> Signed-off-by: Asmaa Mnebhi <Asmaa@mellanox.com>
> ---
>  drivers/char/ipmi/ipmb_dev_int.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_int.c
> index 5720433..285e0b8 100644
> --- a/drivers/char/ipmi/ipmb_dev_int.c
> +++ b/drivers/char/ipmi/ipmb_dev_int.c
> @@ -76,7 +76,7 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size_t count,
>  	struct ipmb_dev *ipmb_dev = to_ipmb_dev(file);
>  	struct ipmb_request_elem *queue_elem;
>  	struct ipmb_msg msg;
> -	ssize_t ret;
> +	ssize_t ret = 0;
>  
>  	memset(&msg, 0, sizeof(msg));
>  
> -- 
> 2.1.2
>