* [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN [not found] <1351940317-14812-1-git-send-email-Julia.Lawall@lip6.fr> @ 2012-11-03 10:58 ` Julia Lawall 2012-11-03 11:30 ` walter harms 2012-11-03 19:43 ` David Miller 2012-11-03 10:58 ` [PATCH 14/16] drivers/ssb/main.c: " Julia Lawall 1 sibling, 2 replies; 7+ messages in thread From: Julia Lawall @ 2012-11-03 10:58 UTC (permalink / raw) To: netdev; +Cc: kernel-janitors, linux-kernel From: Julia Lawall <Julia.Lawall@lip6•fr> Use WARN rather than printk followed by WARN_ON(1), for conciseness. A simplified version of the semantic patch that makes this transformation is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ expression list es; @@ -printk( +WARN(1, es); -WARN_ON(1); // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6•fr> --- drivers/net/ethernet/ibm/emac/mal.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c index 479e43e..84c6b6c 100644 --- a/drivers/net/ethernet/ibm/emac/mal.c +++ b/drivers/net/ethernet/ibm/emac/mal.c @@ -738,13 +738,11 @@ static int __devexit mal_remove(struct platform_device *ofdev) /* Synchronize with scheduled polling */ napi_disable(&mal->napi); - if (!list_empty(&mal->list)) { + if (!list_empty(&mal->list)) /* This is *very* bad */ - printk(KERN_EMERG + WARN(1, KERN_EMERG "mal%d: commac list is not empty on remove!\n", mal->index); - WARN_ON(1); - } dev_set_drvdata(&ofdev->dev, NULL); ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN 2012-11-03 10:58 ` [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN Julia Lawall @ 2012-11-03 11:30 ` walter harms 2012-11-03 14:14 ` Julia Lawall 2012-11-03 19:43 ` David Miller 1 sibling, 1 reply; 7+ messages in thread From: walter harms @ 2012-11-03 11:30 UTC (permalink / raw) To: Julia Lawall; +Cc: netdev, kernel-janitors, linux-kernel Am 03.11.2012 11:58, schrieb Julia Lawall: > From: Julia Lawall <Julia.Lawall@lip6•fr> > > Use WARN rather than printk followed by WARN_ON(1), for conciseness. > > A simplified version of the semantic patch that makes this transformation > is as follows: (http://coccinelle.lip6.fr/) > > // <smpl> > @@ > expression list es; > @@ > > -printk( > +WARN(1, > es); > -WARN_ON(1); > // </smpl> > > Signed-off-by: Julia Lawall <Julia.Lawall@lip6•fr> > > --- > drivers/net/ethernet/ibm/emac/mal.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c > index 479e43e..84c6b6c 100644 > --- a/drivers/net/ethernet/ibm/emac/mal.c > +++ b/drivers/net/ethernet/ibm/emac/mal.c > @@ -738,13 +738,11 @@ static int __devexit mal_remove(struct platform_device *ofdev) > /* Synchronize with scheduled polling */ > napi_disable(&mal->napi); > > - if (!list_empty(&mal->list)) { > + if (!list_empty(&mal->list)) > /* This is *very* bad */ > - printk(KERN_EMERG > + WARN(1, KERN_EMERG > "mal%d: commac list is not empty on remove!\n", > mal->index); > - WARN_ON(1); > - } > > dev_set_drvdata(&ofdev->dev, NULL); > > Hi Julia, you are removing the {} behin the if. I prefer to be a bit conservative about {}. There is suggest to keep them because WARN may be expanded in future (with a second line) and that will cause subtle changes that do no break the code. (Yes i know it is possible to write macros that contain savely more than one line.) re, wh ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN 2012-11-03 11:30 ` walter harms @ 2012-11-03 14:14 ` Julia Lawall 2012-11-03 16:26 ` walter harms 0 siblings, 1 reply; 7+ messages in thread From: Julia Lawall @ 2012-11-03 14:14 UTC (permalink / raw) To: walter harms; +Cc: Julia Lawall, netdev, kernel-janitors, linux-kernel On Sat, 3 Nov 2012, walter harms wrote: > > > Am 03.11.2012 11:58, schrieb Julia Lawall: >> From: Julia Lawall <Julia.Lawall@lip6•fr> >> >> Use WARN rather than printk followed by WARN_ON(1), for conciseness. >> >> A simplified version of the semantic patch that makes this transformation >> is as follows: (http://coccinelle.lip6.fr/) >> >> // <smpl> >> @@ >> expression list es; >> @@ >> >> -printk( >> +WARN(1, >> es); >> -WARN_ON(1); >> // </smpl> >> >> Signed-off-by: Julia Lawall <Julia.Lawall@lip6•fr> >> >> --- >> drivers/net/ethernet/ibm/emac/mal.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/ibm/emac/mal.c b/drivers/net/ethernet/ibm/emac/mal.c >> index 479e43e..84c6b6c 100644 >> --- a/drivers/net/ethernet/ibm/emac/mal.c >> +++ b/drivers/net/ethernet/ibm/emac/mal.c >> @@ -738,13 +738,11 @@ static int __devexit mal_remove(struct platform_device *ofdev) >> /* Synchronize with scheduled polling */ >> napi_disable(&mal->napi); >> >> - if (!list_empty(&mal->list)) { >> + if (!list_empty(&mal->list)) >> /* This is *very* bad */ >> - printk(KERN_EMERG >> + WARN(1, KERN_EMERG >> "mal%d: commac list is not empty on remove!\n", >> mal->index); >> - WARN_ON(1); >> - } >> >> dev_set_drvdata(&ofdev->dev, NULL); >> >> > > Hi Julia, > you are removing the {} behin the if. I prefer to be a bit conservative > about {}. There is suggest to keep them because WARN may be expanded in > future (with a second line) and that will cause subtle changes that do > no break the code. (Yes i know it is possible to write macros that > contain savely more than one line.) WARN is already multi-line, surrounded by ({ }). It seems to be set up to be used as an expression. Is it necessary to assume that it might someday be changed from safe to unsafe? julia ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN 2012-11-03 14:14 ` Julia Lawall @ 2012-11-03 16:26 ` walter harms 2012-11-03 16:35 ` Julia Lawall 0 siblings, 1 reply; 7+ messages in thread From: walter harms @ 2012-11-03 16:26 UTC (permalink / raw) To: Julia Lawall; +Cc: netdev, kernel-janitors, linux-kernel Am 03.11.2012 15:14, schrieb Julia Lawall: > On Sat, 3 Nov 2012, walter harms wrote: > >> >> >> Am 03.11.2012 11:58, schrieb Julia Lawall: >>> From: Julia Lawall <Julia.Lawall@lip6•fr> >>> >>> Use WARN rather than printk followed by WARN_ON(1), for conciseness. >>> >>> A simplified version of the semantic patch that makes this >>> transformation >>> is as follows: (http://coccinelle.lip6.fr/) >>> >>> // <smpl> >>> @@ >>> expression list es; >>> @@ >>> >>> -printk( >>> +WARN(1, >>> es); >>> -WARN_ON(1); >>> // </smpl> >>> >>> Signed-off-by: Julia Lawall <Julia.Lawall@lip6•fr> >>> >>> --- >>> drivers/net/ethernet/ibm/emac/mal.c | 6 ++---- >>> 1 file changed, 2 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/ibm/emac/mal.c >>> b/drivers/net/ethernet/ibm/emac/mal.c >>> index 479e43e..84c6b6c 100644 >>> --- a/drivers/net/ethernet/ibm/emac/mal.c >>> +++ b/drivers/net/ethernet/ibm/emac/mal.c >>> @@ -738,13 +738,11 @@ static int __devexit mal_remove(struct >>> platform_device *ofdev) >>> /* Synchronize with scheduled polling */ >>> napi_disable(&mal->napi); >>> >>> - if (!list_empty(&mal->list)) { >>> + if (!list_empty(&mal->list)) >>> /* This is *very* bad */ >>> - printk(KERN_EMERG >>> + WARN(1, KERN_EMERG >>> "mal%d: commac list is not empty on remove!\n", >>> mal->index); >>> - WARN_ON(1); >>> - } >>> >>> dev_set_drvdata(&ofdev->dev, NULL); >>> >>> >> >> Hi Julia, >> you are removing the {} behin the if. I prefer to be a bit conservative >> about {}. There is suggest to keep them because WARN may be expanded in >> future (with a second line) and that will cause subtle changes that do >> no break the code. (Yes i know it is possible to write macros that >> contain savely more than one line.) > > WARN is already multi-line, surrounded by ({ }). It seems to be set up > to be used as an expression. Is it necessary to assume that it might > someday be changed from safe to unsafe? > my bad, NTL looks like a candidate for a function. While looking i have noticed that a lot of drivers define there private "assert" macro. It is very similar to warn. (e.g.) #define RTL819x_DEBUG #ifdef RTL819x_DEBUG #define assert(expr) \ if (!(expr)) { \ printk( "Assertion failed! %s,%s,%s,line=%d\n", \ #expr,__FILE__,__FUNCTION__,__LINE__); \ } re, wh ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN 2012-11-03 16:26 ` walter harms @ 2012-11-03 16:35 ` Julia Lawall 0 siblings, 0 replies; 7+ messages in thread From: Julia Lawall @ 2012-11-03 16:35 UTC (permalink / raw) To: walter harms; +Cc: Julia Lawall, netdev, kernel-janitors, linux-kernel > While looking i have noticed that a lot of drivers define there private "assert" macro. > It is very similar to warn. > > (e.g.) > #define RTL819x_DEBUG > #ifdef RTL819x_DEBUG > #define assert(expr) \ > if (!(expr)) { \ > printk( "Assertion failed! %s,%s,%s,line=%d\n", \ > #expr,__FILE__,__FUNCTION__,__LINE__); \ > } WARN is more complicated. At least with the right debugging options turned on, it dumps the stack, via warn_slowpath_common. julia ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN 2012-11-03 10:58 ` [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN Julia Lawall 2012-11-03 11:30 ` walter harms @ 2012-11-03 19:43 ` David Miller 1 sibling, 0 replies; 7+ messages in thread From: David Miller @ 2012-11-03 19:43 UTC (permalink / raw) To: Julia.Lawall; +Cc: netdev, kernel-janitors, linux-kernel From: Julia Lawall <Julia.Lawall@lip6•fr> Date: Sat, 3 Nov 2012 11:58:31 +0100 > From: Julia Lawall <Julia.Lawall@lip6•fr> > > Use WARN rather than printk followed by WARN_ON(1), for conciseness. > > A simplified version of the semantic patch that makes this transformation > is as follows: (http://coccinelle.lip6.fr/) ... > Signed-off-by: Julia Lawall <Julia.Lawall@lip6•fr> Applied. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 14/16] drivers/ssb/main.c: use WARN [not found] <1351940317-14812-1-git-send-email-Julia.Lawall@lip6.fr> 2012-11-03 10:58 ` [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN Julia Lawall @ 2012-11-03 10:58 ` Julia Lawall 1 sibling, 0 replies; 7+ messages in thread From: Julia Lawall @ 2012-11-03 10:58 UTC (permalink / raw) To: Michael Buesch; +Cc: kernel-janitors, netdev, linux-kernel From: Julia Lawall <Julia.Lawall@lip6•fr> Use WARN rather than printk followed by WARN_ON(1), for conciseness. A simplified version of the semantic patch that makes this transformation is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ expression list es; @@ -printk( +WARN(1, es); -WARN_ON(1); // </smpl> Signed-off-by: Julia Lawall <Julia.Lawall@lip6•fr> --- drivers/ssb/main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c index df0f145..519688d 100644 --- a/drivers/ssb/main.c +++ b/drivers/ssb/main.c @@ -1118,8 +1118,7 @@ static u32 ssb_tmslow_reject_bitmask(struct ssb_device *dev) case SSB_IDLOW_SSBREV_27: /* same here */ return SSB_TMSLOW_REJECT; /* this is a guess */ default: - printk(KERN_INFO "ssb: Backplane Revision 0x%.8X\n", rev); - WARN_ON(1); + WARN(1, KERN_INFO "ssb: Backplane Revision 0x%.8X\n", rev); } return (SSB_TMSLOW_REJECT | SSB_TMSLOW_REJECT_23); } ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-11-03 19:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1351940317-14812-1-git-send-email-Julia.Lawall@lip6.fr>
2012-11-03 10:58 ` [PATCH 10/16] drivers/net/ethernet/ibm/emac/mal.c: use WARN Julia Lawall
2012-11-03 11:30 ` walter harms
2012-11-03 14:14 ` Julia Lawall
2012-11-03 16:26 ` walter harms
2012-11-03 16:35 ` Julia Lawall
2012-11-03 19:43 ` David Miller
2012-11-03 10:58 ` [PATCH 14/16] drivers/ssb/main.c: " Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox