All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [media] stk1160: Remove dead code from stk1160_i2c_read_reg()
@ 2008-11-09 17:04 Jesper Juhl
  2012-10-13  3:54 ` Ezequiel Garcia
  0 siblings, 1 reply; 9+ messages in thread
From: Jesper Juhl @ 2008-11-09 17:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Ezequiel Garcia, linux-kernel

There are two checks for 'rc' being less than zero with no change to
'rc' between the two, so the second is just dead code - remove it.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/media/usb/stk1160/stk1160-i2c.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
index 176ac93..035cf8c 100644
--- a/drivers/media/usb/stk1160/stk1160-i2c.c
+++ b/drivers/media/usb/stk1160/stk1160-i2c.c
@@ -117,9 +117,6 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
 		return rc;
 
 	stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
-	if (rc < 0)
-		return rc;
-
 	return 0;
 }
 
-- 
1.7.1


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] [media] stk1160: Remove dead code from stk1160_i2c_read_reg()
  2008-11-09 17:04 [PATCH] [media] stk1160: Remove dead code from stk1160_i2c_read_reg() Jesper Juhl
@ 2012-10-13  3:54 ` Ezequiel Garcia
  2012-10-15 18:26   ` Jesper Juhl
  0 siblings, 1 reply; 9+ messages in thread
From: Ezequiel Garcia @ 2012-10-13  3:54 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Sun, Nov 9, 2008 at 2:04 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> There are two checks for 'rc' being less than zero with no change to
> 'rc' between the two, so the second is just dead code - remove it.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/media/usb/stk1160/stk1160-i2c.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
> index 176ac93..035cf8c 100644
> --- a/drivers/media/usb/stk1160/stk1160-i2c.c
> +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
> @@ -117,9 +117,6 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
>                 return rc;
>
>         stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> -       if (rc < 0)
> -               return rc;
> -
>         return 0;
>  }
>

Thanks for doing this. Wouldn't you like to save stk1160_read_reg
return code to rc, instead of this?

    Ezequiel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [media] stk1160: Remove dead code from stk1160_i2c_read_reg()
  2012-10-13  3:54 ` Ezequiel Garcia
@ 2012-10-15 18:26   ` Jesper Juhl
  2012-10-15 22:52     ` [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg() Jesper Juhl
  0 siblings, 1 reply; 9+ messages in thread
From: Jesper Juhl @ 2012-10-15 18:26 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Sat, 13 Oct 2012, Ezequiel Garcia wrote:

> On Sun, Nov 9, 2008 at 2:04 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > There are two checks for 'rc' being less than zero with no change to
> > 'rc' between the two, so the second is just dead code - remove it.
> >
> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > ---
> >  drivers/media/usb/stk1160/stk1160-i2c.c |    3 ---
> >  1 files changed, 0 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
> > index 176ac93..035cf8c 100644
> > --- a/drivers/media/usb/stk1160/stk1160-i2c.c
> > +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
> > @@ -117,9 +117,6 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
> >                 return rc;
> >
> >         stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> > -       if (rc < 0)
> > -               return rc;
> > -
> >         return 0;
> >  }
> >
> 
> Thanks for doing this. Wouldn't you like to save stk1160_read_reg
> return code to rc, instead of this?
> 
Ahh yes, I guess I was too quick to just assume it was dead code. 
Looking at it again; what you suggest must have been the original 
intention. I'll cook up a new patch.

Thanks.

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
  2012-10-15 18:26   ` Jesper Juhl
@ 2012-10-15 22:52     ` Jesper Juhl
  2012-10-15 23:14       ` Ezequiel Garcia
  0 siblings, 1 reply; 9+ messages in thread
From: Jesper Juhl @ 2012-10-15 22:52 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Mon, 15 Oct 2012, Jesper Juhl wrote:

> On Sat, 13 Oct 2012, Ezequiel Garcia wrote:
> 
> > On Sun, Nov 9, 2008 at 2:04 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > > There are two checks for 'rc' being less than zero with no change to
> > > 'rc' between the two, so the second is just dead code - remove it.
> > >
> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > > ---
> > >  drivers/media/usb/stk1160/stk1160-i2c.c |    3 ---
> > >  1 files changed, 0 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
> > > index 176ac93..035cf8c 100644
> > > --- a/drivers/media/usb/stk1160/stk1160-i2c.c
> > > +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
> > > @@ -117,9 +117,6 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
> > >                 return rc;
> > >
> > >         stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> > > -       if (rc < 0)
> > > -               return rc;
> > > -
> > >         return 0;
> > >  }
> > >
> > 
> > Thanks for doing this. Wouldn't you like to save stk1160_read_reg
> > return code to rc, instead of this?
> > 
> Ahh yes, I guess I was too quick to just assume it was dead code. 
> Looking at it again; what you suggest must have been the original 
> intention. I'll cook up a new patch.
> 
> Thanks.
> 
From: Jesper Juhl <jj@chaosbits.net>
Date: Sat, 13 Oct 2012 00:16:37 +0200
Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()

Currently there are two checks for 'rc' being less than zero with no
change to 'rc' between the two, so the second is just dead code.
The intention seems to have been to assign the return value of
'stk1160_read_reg()' to 'rc' before the (currently dead) second check
and then test /that/. This patch does that.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/media/usb/stk1160/stk1160-i2c.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
index 176ac93..a2370e4 100644
--- a/drivers/media/usb/stk1160/stk1160-i2c.c
+++ b/drivers/media/usb/stk1160/stk1160-i2c.c
@@ -116,10 +116,9 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
 	if (rc < 0)
 		return rc;
 
-	stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
+	rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
 	if (rc < 0)
 		return rc;
-
 	return 0;
 }
 
-- 
1.7.1


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
  2012-10-15 22:52     ` [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg() Jesper Juhl
@ 2012-10-15 23:14       ` Ezequiel Garcia
  2012-10-16  0:03         ` Jesper Juhl
  0 siblings, 1 reply; 9+ messages in thread
From: Ezequiel Garcia @ 2012-10-15 23:14 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Mon, Oct 15, 2012 at 7:52 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> On Mon, 15 Oct 2012, Jesper Juhl wrote:
>
>> On Sat, 13 Oct 2012, Ezequiel Garcia wrote:
>>
>> > On Sun, Nov 9, 2008 at 2:04 PM, Jesper Juhl <jj@chaosbits.net> wrote:
>> > > There are two checks for 'rc' being less than zero with no change to
>> > > 'rc' between the two, so the second is just dead code - remove it.
>> > >
>> > > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
>> > > ---
>> > >  drivers/media/usb/stk1160/stk1160-i2c.c |    3 ---
>> > >  1 files changed, 0 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
>> > > index 176ac93..035cf8c 100644
>> > > --- a/drivers/media/usb/stk1160/stk1160-i2c.c
>> > > +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
>> > > @@ -117,9 +117,6 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
>> > >                 return rc;
>> > >
>> > >         stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
>> > > -       if (rc < 0)
>> > > -               return rc;
>> > > -
>> > >         return 0;
>> > >  }
>> > >
>> >
>> > Thanks for doing this. Wouldn't you like to save stk1160_read_reg
>> > return code to rc, instead of this?
>> >
>> Ahh yes, I guess I was too quick to just assume it was dead code.
>> Looking at it again; what you suggest must have been the original
>> intention. I'll cook up a new patch.
>>
>> Thanks.
>>
> From: Jesper Juhl <jj@chaosbits.net>
> Date: Sat, 13 Oct 2012 00:16:37 +0200
> Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
>
> Currently there are two checks for 'rc' being less than zero with no
> change to 'rc' between the two, so the second is just dead code.
> The intention seems to have been to assign the return value of
> 'stk1160_read_reg()' to 'rc' before the (currently dead) second check
> and then test /that/. This patch does that.
>


This is an overly complicated explanation for such a small patch.
Can you try to simplify it?


> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/media/usb/stk1160/stk1160-i2c.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
> index 176ac93..a2370e4 100644
> --- a/drivers/media/usb/stk1160/stk1160-i2c.c
> +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
> @@ -116,10 +116,9 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
>         if (rc < 0)
>                 return rc;
>
> -       stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> +       rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
>         if (rc < 0)
>                 return rc;

Why are you removing this line?


> -
>         return 0;
>  }
>


Thanks,

    Ezequiel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
  2012-10-15 23:14       ` Ezequiel Garcia
@ 2012-10-16  0:03         ` Jesper Juhl
  2012-10-16  2:16           ` Ezequiel Garcia
  0 siblings, 1 reply; 9+ messages in thread
From: Jesper Juhl @ 2012-10-16  0:03 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Mon, 15 Oct 2012, Ezequiel Garcia wrote:

> On Mon, Oct 15, 2012 at 7:52 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > On Mon, 15 Oct 2012, Jesper Juhl wrote:
> >
> >> On Sat, 13 Oct 2012, Ezequiel Garcia wrote:
> >>
[...]
> > Currently there are two checks for 'rc' being less than zero with no
> > change to 'rc' between the two, so the second is just dead code.
> > The intention seems to have been to assign the return value of
> > 'stk1160_read_reg()' to 'rc' before the (currently dead) second check
> > and then test /that/. This patch does that.
> >
> 
> This is an overly complicated explanation for such a small patch.
> Can you try to simplify it?
> 
How's this?


From: Jesper Juhl <jj@chaosbits.net>
Date: Sat, 13 Oct 2012 00:16:37 +0200
Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()

Remember to collect the exit status from 'stk1160_read_reg()' in 'rc' 
before testing it for less than zero.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/media/usb/stk1160/stk1160-i2c.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
index 176ac93..a2370e4 100644
--- a/drivers/media/usb/stk1160/stk1160-i2c.c
+++ b/drivers/media/usb/stk1160/stk1160-i2c.c
@@ -116,10 +116,9 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
 	if (rc < 0)
 		return rc;
 
-	stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
+	rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
 	if (rc < 0)
 		return rc;
-
 	return 0;
 }
 
-- 
1.7.1


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
  2012-10-16  0:03         ` Jesper Juhl
@ 2012-10-16  2:16           ` Ezequiel Garcia
  2012-10-16 19:06             ` Jesper Juhl
  0 siblings, 1 reply; 9+ messages in thread
From: Ezequiel Garcia @ 2012-10-16  2:16 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Mon, Oct 15, 2012 at 9:03 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> On Mon, 15 Oct 2012, Ezequiel Garcia wrote:
>
>> On Mon, Oct 15, 2012 at 7:52 PM, Jesper Juhl <jj@chaosbits.net> wrote:
>> > On Mon, 15 Oct 2012, Jesper Juhl wrote:
>> >
>> >> On Sat, 13 Oct 2012, Ezequiel Garcia wrote:
>> >>
> [...]
>> > Currently there are two checks for 'rc' being less than zero with no
>> > change to 'rc' between the two, so the second is just dead code.
>> > The intention seems to have been to assign the return value of
>> > 'stk1160_read_reg()' to 'rc' before the (currently dead) second check
>> > and then test /that/. This patch does that.
>> >
>>
>> This is an overly complicated explanation for such a small patch.
>> Can you try to simplify it?
>>
> How's this?
>
>
> From: Jesper Juhl <jj@chaosbits.net>
> Date: Sat, 13 Oct 2012 00:16:37 +0200
> Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
>
> Remember to collect the exit status from 'stk1160_read_reg()' in 'rc'
> before testing it for less than zero.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/media/usb/stk1160/stk1160-i2c.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
> index 176ac93..a2370e4 100644
> --- a/drivers/media/usb/stk1160/stk1160-i2c.c
> +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
> @@ -116,10 +116,9 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
>         if (rc < 0)
>                 return rc;
>
> -       stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> +       rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
>         if (rc < 0)
>                 return rc;
> -
^^^^^^^^^^^^^^^^^^^^^^^^
Sorry for the nitpick, but I'd like you to *not* remove this line.

Thanks

    Ezequiel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
  2012-10-16  2:16           ` Ezequiel Garcia
@ 2012-10-16 19:06             ` Jesper Juhl
  2012-10-17 12:39               ` Ezequiel Garcia
  0 siblings, 1 reply; 9+ messages in thread
From: Jesper Juhl @ 2012-10-16 19:06 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Mon, 15 Oct 2012, Ezequiel Garcia wrote:

> On Mon, Oct 15, 2012 at 9:03 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > On Mon, 15 Oct 2012, Ezequiel Garcia wrote:
> >
> >> On Mon, Oct 15, 2012 at 7:52 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> >> > On Mon, 15 Oct 2012, Jesper Juhl wrote:
> >> >
> >> >> On Sat, 13 Oct 2012, Ezequiel Garcia wrote:
> >> >>
> > [...]
> >> > Currently there are two checks for 'rc' being less than zero with no
> >> > change to 'rc' between the two, so the second is just dead code.
> >> > The intention seems to have been to assign the return value of
> >> > 'stk1160_read_reg()' to 'rc' before the (currently dead) second check
> >> > and then test /that/. This patch does that.
> >> >
> >>
> >> This is an overly complicated explanation for such a small patch.
> >> Can you try to simplify it?
> >>
> > How's this?
> >
> >
> > From: Jesper Juhl <jj@chaosbits.net>
> > Date: Sat, 13 Oct 2012 00:16:37 +0200
> > Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
> >
> > Remember to collect the exit status from 'stk1160_read_reg()' in 'rc'
> > before testing it for less than zero.
> >
> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> > ---
> >  drivers/media/usb/stk1160/stk1160-i2c.c |    3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
> > index 176ac93..a2370e4 100644
> > --- a/drivers/media/usb/stk1160/stk1160-i2c.c
> > +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
> > @@ -116,10 +116,9 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
> >         if (rc < 0)
> >                 return rc;
> >
> > -       stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> > +       rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> >         if (rc < 0)
> >                 return rc;
> > -
> ^^^^^^^^^^^^^^^^^^^^^^^^
> Sorry for the nitpick, but I'd like you to *not* remove this line.
> 
No problem.
I hope the below is OK :-)


From: Jesper Juhl <jj@chaosbits.net>
Date: Sat, 13 Oct 2012 00:16:37 +0200
Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()

Remember to collect the exit status from 'stk1160_read_reg()' in 'rc'
before testing it for less than zero.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/media/usb/stk1160/stk1160-i2c.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
index 176ac93..850cf28 100644
--- a/drivers/media/usb/stk1160/stk1160-i2c.c
+++ b/drivers/media/usb/stk1160/stk1160-i2c.c
@@ -116,7 +116,7 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
 	if (rc < 0)
 		return rc;
 
-	stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
+	rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
 	if (rc < 0)
 		return rc;
 
-- 
1.7.1


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
  2012-10-16 19:06             ` Jesper Juhl
@ 2012-10-17 12:39               ` Ezequiel Garcia
  0 siblings, 0 replies; 9+ messages in thread
From: Ezequiel Garcia @ 2012-10-17 12:39 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: Mauro Carvalho Chehab, linux-media, linux-kernel

On Tue, Oct 16, 2012 at 4:06 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> On Mon, 15 Oct 2012, Ezequiel Garcia wrote:
>
>> On Mon, Oct 15, 2012 at 9:03 PM, Jesper Juhl <jj@chaosbits.net> wrote:
>> > On Mon, 15 Oct 2012, Ezequiel Garcia wrote:
>> >
>> >> On Mon, Oct 15, 2012 at 7:52 PM, Jesper Juhl <jj@chaosbits.net> wrote:
>> >> > On Mon, 15 Oct 2012, Jesper Juhl wrote:
>> >> >
>> >> >> On Sat, 13 Oct 2012, Ezequiel Garcia wrote:
>> >> >>
>> > [...]
>> >> > Currently there are two checks for 'rc' being less than zero with no
>> >> > change to 'rc' between the two, so the second is just dead code.
>> >> > The intention seems to have been to assign the return value of
>> >> > 'stk1160_read_reg()' to 'rc' before the (currently dead) second check
>> >> > and then test /that/. This patch does that.
>> >> >
>> >>
>> >> This is an overly complicated explanation for such a small patch.
>> >> Can you try to simplify it?
>> >>
>> > How's this?
>> >
>> >
>> > From: Jesper Juhl <jj@chaosbits.net>
>> > Date: Sat, 13 Oct 2012 00:16:37 +0200
>> > Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
>> >
>> > Remember to collect the exit status from 'stk1160_read_reg()' in 'rc'
>> > before testing it for less than zero.
>> >
>> > Signed-off-by: Jesper Juhl <jj@chaosbits.net>
>> > ---
>> >  drivers/media/usb/stk1160/stk1160-i2c.c |    3 +--
>> >  1 files changed, 1 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
>> > index 176ac93..a2370e4 100644
>> > --- a/drivers/media/usb/stk1160/stk1160-i2c.c
>> > +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
>> > @@ -116,10 +116,9 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
>> >         if (rc < 0)
>> >                 return rc;
>> >
>> > -       stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
>> > +       rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
>> >         if (rc < 0)
>> >                 return rc;
>> > -
>> ^^^^^^^^^^^^^^^^^^^^^^^^
>> Sorry for the nitpick, but I'd like you to *not* remove this line.
>>
> No problem.
> I hope the below is OK :-)
>
>
> From: Jesper Juhl <jj@chaosbits.net>
> Date: Sat, 13 Oct 2012 00:16:37 +0200
> Subject: [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg()
>
> Remember to collect the exit status from 'stk1160_read_reg()' in 'rc'
> before testing it for less than zero.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  drivers/media/usb/stk1160/stk1160-i2c.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/usb/stk1160/stk1160-i2c.c b/drivers/media/usb/stk1160/stk1160-i2c.c
> index 176ac93..850cf28 100644
> --- a/drivers/media/usb/stk1160/stk1160-i2c.c
> +++ b/drivers/media/usb/stk1160/stk1160-i2c.c
> @@ -116,7 +116,7 @@ static int stk1160_i2c_read_reg(struct stk1160 *dev, u8 addr,
>         if (rc < 0)
>                 return rc;
>
> -       stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
> +       rc = stk1160_read_reg(dev, STK1160_SBUSR_RD, value);
>         if (rc < 0)
>                 return rc;
>
> --
> 1.7.1
>
>
> --
> Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
>

Acked-by: Ezequiel Garcia <elezegarcia@gmail.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-10-17 12:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-09 17:04 [PATCH] [media] stk1160: Remove dead code from stk1160_i2c_read_reg() Jesper Juhl
2012-10-13  3:54 ` Ezequiel Garcia
2012-10-15 18:26   ` Jesper Juhl
2012-10-15 22:52     ` [PATCH] [media] stk1160: Check return value of stk1160_read_reg() in stk1160_i2c_read_reg() Jesper Juhl
2012-10-15 23:14       ` Ezequiel Garcia
2012-10-16  0:03         ` Jesper Juhl
2012-10-16  2:16           ` Ezequiel Garcia
2012-10-16 19:06             ` Jesper Juhl
2012-10-17 12:39               ` Ezequiel Garcia

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.