Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1481754 > unrolled thread

Re: [PATCH] skd: fix assert typo

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] skd: fix assert typo Jeff Moyer <jmoyer@redhat.com> - 2016-09-12 19:50 +0200
    [PATCH v2] skd: fix assert typo Eric Engestrom <eric.engestrom@imgtec.com> - 2016-09-16 11:40 +0200
      Re: [PATCH v2] skd: fix assert typo Johannes Thumshirn <jthumshirn@suse.de> - 2016-09-16 14:10 +0200
      Re: [PATCH v2] skd: fix assert typo Jeff Moyer <jmoyer@redhat.com> - 2016-09-16 14:50 +0200
        Re: [PATCH v2] skd: fix assert typo Eric Engestrom <eric.engestrom@imgtec.com> - 2016-09-16 15:10 +0200

#1481754 — Re: [PATCH] skd: fix assert typo

Eric Engestrom <eric.engestrom@imgtec.com> writes:

> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---
>  drivers/block/skd_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
> index 3822eae..94a7425 100644
> --- a/drivers/block/skd_main.c
> +++ b/drivers/block/skd_main.c
> @@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
>  		break;
>  
>  	default:
> -		SKD_ASSERT("Don't know what to send");
> +		SKD_ASSERT(!"Don't know what to send");

That's perverse.  Just change SKD_ASSERT to pr_err and Robert's your
mother's brother.

Cheers,
Jeff

[toc] | [next] | [standalone]

#1484840 — [PATCH v2] skd: fix assert typo

The assert was missing a `!` to become active, but since that would only turn
it into a complicated codepath for a pr_err(), let's simply replace it.

CC: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
 drivers/block/skd_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 3822eae..c04e92a 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
 		break;
 
 	default:
-		SKD_ASSERT("Don't know what to send");
+		pr_err("Don't know what to send");
 		return;
 
 	}
@@ -2105,7 +2105,7 @@ static void skd_complete_internal(struct skd_device *skdev,
 		break;
 
 	default:
-		SKD_ASSERT("we didn't send this");
+		pr_err("we didn't send this");
 	}
 }
 
-- 
Cheers,
  Eric

[toc] | [prev] | [next] | [standalone]

#1484928 — Re: [PATCH v2] skd: fix assert typo

On Fri, Sep 16, 2016 at 10:31:23AM +0100, Eric Engestrom wrote:
> The assert was missing a `!` to become active, but since that would only turn
> it into a complicated codepath for a pr_err(), let's simply replace it.
> 
> CC: Jeff Moyer <jmoyer@redhat.com>
> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---

Looks ok,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[toc] | [prev] | [next] | [standalone]

#1484964 — Re: [PATCH v2] skd: fix assert typo

Eric Engestrom <eric.engestrom@imgtec.com> writes:

> The assert was missing a `!` to become active, but since that would only turn
> it into a complicated codepath for a pr_err(), let's simply replace it.

The skd assert macro prints out file and line number, which would have
been nice to keep.  Sorry I didn't explicitly mention that last time.

-Jeff

>
> CC: Jeff Moyer <jmoyer@redhat.com>
> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---
>  drivers/block/skd_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
> index 3822eae..c04e92a 100644
> --- a/drivers/block/skd_main.c
> +++ b/drivers/block/skd_main.c
> @@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
>  		break;
>  
>  	default:
> -		SKD_ASSERT("Don't know what to send");
> +		pr_err("Don't know what to send");
>  		return;
>  
>  	}
> @@ -2105,7 +2105,7 @@ static void skd_complete_internal(struct skd_device *skdev,
>  		break;
>  
>  	default:
> -		SKD_ASSERT("we didn't send this");
> +		pr_err("we didn't send this");
>  	}
>  }

[toc] | [prev] | [next] | [standalone]

#1484981 — Re: [PATCH v2] skd: fix assert typo

On Fri, Sep 16, 2016 at 08:40:54AM -0400, Jeff Moyer wrote:
> Eric Engestrom <eric.engestrom@imgtec.com> writes:
> 
> > The assert was missing a `!` to become active, but since that would only turn
> > it into a complicated codepath for a pr_err(), let's simply replace it.
> 
> The skd assert macro prints out file and line number, which would have
> been nice to keep.  Sorry I didn't explicitly mention that last time.

I'm not a fan of duplicating code, so if you want to keep the SKD_ASSERT()
format, my suggestion would be to go with the v1 of this patch. It might
end up having an `if` that will always be true, but the compiler will most
likely take care of optimising that away :)

Cheers,
  Eric

> 
> -Jeff
> 
> >
> > CC: Jeff Moyer <jmoyer@redhat.com>
> > Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> > ---
> >  drivers/block/skd_main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
> > index 3822eae..c04e92a 100644
> > --- a/drivers/block/skd_main.c
> > +++ b/drivers/block/skd_main.c
> > @@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
> >  		break;
> >  
> >  	default:
> > -		SKD_ASSERT("Don't know what to send");
> > +		pr_err("Don't know what to send");
> >  		return;
> >  
> >  	}
> > @@ -2105,7 +2105,7 @@ static void skd_complete_internal(struct skd_device *skdev,
> >  		break;
> >  
> >  	default:
> > -		SKD_ASSERT("we didn't send this");
> > +		pr_err("we didn't send this");
> >  	}
> >  }

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web