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

From: Asmaa Mnebhi
Date: Wed Jul 24 2019 - 15:09:18 EST


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

-----Original Message-----
From: Corey Minyard <tcminyard@xxxxxxxxx> On Behalf Of Corey Minyard
Sent: Wednesday, July 24, 2019 3:07 PM
To: Asmaa Mnebhi <Asmaa@xxxxxxxxxxxx>
Cc: linux-kernel@xxxxxxxxxxxxxxx
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@xxxxxxxxxxxx>
> Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>

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
>