[Qemu-devel] [PATCH] spapr_vscsi: Set uninitialized variable

[Qemu-devel] [PATCH] spapr_vscsi: Set uninitialized variable

[Stefan Weil]

cppcheck reports this error:

hw/spapr_vscsi.c:274: error: Uninitialized variable: rc

If llen == 0, rc was indeed used without being initialized.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hw/spapr_vscsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index e142dae..9928334 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -255,7 +255,7 @@ static int vscsi_srp_direct_data(VSCSIState *s, vscsi_req *req,
 {
     struct srp_direct_buf *md = req->cur_desc;
     uint32_t llen;
-    int rc;
+    int rc = 0;
 
     dprintf("VSCSI: direct segment 0x%x bytes, va=0x%llx desc len=0x%x\n",
             len, (unsigned long long)md->va, md->len);
-- 
1.7.2.5

[Qemu-devel] Re: [PATCH] spapr_vscsi: Set uninitialized variable

[Alexander Graf]

On 04/03/2011 06:21 PM, Stefan Weil wrote:
> cppcheck reports this error:
>
> hw/spapr_vscsi.c:274: error: Uninitialized variable: rc
>
> If llen == 0, rc was indeed used without being initialized.
>
> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
> ---
>   hw/spapr_vscsi.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index e142dae..9928334 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -255,7 +255,7 @@ static int vscsi_srp_direct_data(VSCSIState *s, vscsi_req *req,
>   {
>       struct srp_direct_buf *md = req->cur_desc;
>       uint32_t llen;
> -    int rc;
> +    int rc = 0;

David, is this correct? Or would rc be -1 when !llen?


Alex

[Qemu-devel] Re: [PATCH] spapr_vscsi: Set uninitialized variable

[David Gibson]

On Mon, Apr 04, 2011 at 05:03:44PM +0200, Alexander Graf wrote:
> On 04/03/2011 06:21 PM, Stefan Weil wrote:
> >cppcheck reports this error:
> >
> >hw/spapr_vscsi.c:274: error: Uninitialized variable: rc
> >
> >If llen == 0, rc was indeed used without being initialized.
> >
> >Signed-off-by: Stefan Weil<weil@mail.berlios.de>
> >---
> >  hw/spapr_vscsi.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> >index e142dae..9928334 100644
> >--- a/hw/spapr_vscsi.c
> >+++ b/hw/spapr_vscsi.c
> >@@ -255,7 +255,7 @@ static int vscsi_srp_direct_data(VSCSIState *s, vscsi_req *req,
> >  {
> >      struct srp_direct_buf *md = req->cur_desc;
> >      uint32_t llen;
> >-    int rc;
> >+    int rc = 0;
> 
> David, is this correct? Or would rc be -1 when !llen?

I talked to Ben, who wrote this code - apparently his mail server blew
up.  This patch should be correct, AFAWCT.  It's not totally clear
what the right return value should be in thie case, or indeed that
this case ever actually happens, but we think 0 is right.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[Qemu-devel] Re: [PATCH] spapr_vscsi: Set uninitialized variable

[Benjamin Herrenschmidt]

On Tue, 2011-04-05 at 14:14 +1000, David Gibson wrote:
> > >@@ -255,7 +255,7 @@ static int vscsi_srp_direct_data(VSCSIState *s,
> vscsi_req *req,
> > >  {
> > >      struct srp_direct_buf *md = req->cur_desc;
> > >      uint32_t llen;
> > >-    int rc;
> > >+    int rc = 0;
> > 
> > David, is this correct? Or would rc be -1 when !llen?
> 
> I talked to Ben, who wrote this code - apparently his mail server blew
> up.  This patch should be correct, AFAWCT.  It's not totally clear
> what the right return value should be in thie case, or indeed that
> this case ever actually happens, but we think 0 is right. 

Just double checked and we never call that function if len is 0

Cheers,
Ben.

[Qemu-devel] Re: [PATCH] spapr_vscsi: Set uninitialized variable

[Alexander Graf]

On 05.04.2011, at 07:04, Benjamin Herrenschmidt wrote:

> 
> On Tue, 2011-04-05 at 14:14 +1000, David Gibson wrote:
>>>> @@ -255,7 +255,7 @@ static int vscsi_srp_direct_data(VSCSIState *s,
>> vscsi_req *req,
>>>> {
>>>>     struct srp_direct_buf *md = req->cur_desc;
>>>>     uint32_t llen;
>>>> -    int rc;
>>>> +    int rc = 0;
>>> 
>>> David, is this correct? Or would rc be -1 when !llen?
>> 
>> I talked to Ben, who wrote this code - apparently his mail server blew
>> up.  This patch should be correct, AFAWCT.  It's not totally clear
>> what the right return value should be in thie case, or indeed that
>> this case ever actually happens, but we think 0 is right. 
> 
> Just double checked and we never call that function if len is 0

That's probably the reason gcc didn't complain before. So I'll just take Stefan's patch.


Alex