* Re: [PATCH 1/2] Staging: vme: remove unreachable code [not found] ` <1298403376-28352-2-git-send-email-manohar.vanga@cern.ch> @ 2011-02-23 9:19 ` Martyn Welch 2011-02-23 12:17 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Martyn Welch @ 2011-02-23 9:19 UTC (permalink / raw) To: Manohar Vanga; +Cc: cota, devel, linux-kernel, Greg KH On 22/02/11 19:36, Manohar Vanga wrote: > Remove unreachable code from vme_register_bridge > > Signed-off-by: Manohar Vanga <manohar.vanga@cern.ch> Yeah - that's there from development. If the function needed to be extended, that's the next part of the error path. Not sure how leaving lines like this are viewed in the kernel code, I'm happy for this to be removed if it's not considered good practice. In that case: Acked-by: Martyn Welch <martyn.welch@ge.com> Martyn > --- > drivers/staging/vme/vme.c | 1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/drivers/staging/vme/vme.c b/drivers/staging/vme/vme.c > index d9fc864..88bf455 100644 > --- a/drivers/staging/vme/vme.c > +++ b/drivers/staging/vme/vme.c > @@ -1363,7 +1363,6 @@ int vme_register_bridge(struct vme_bridge *bridge) > > return retval; > > - i = VME_SLOTS_MAX; > err_reg: > while (i > -1) { > dev = &bridge->dev[i]; -- Martyn Welch (Principal Software Engineer) | Registered in England and GE Intelligent Platforms | Wales (3828642) at 100 T +44(0)127322748 | Barbirolli Square, Manchester, E martyn.welch@ge.com | M2 3AB VAT:GB 927559189 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Staging: vme: remove unreachable code 2011-02-23 9:19 ` [PATCH 1/2] Staging: vme: remove unreachable code Martyn Welch @ 2011-02-23 12:17 ` Dan Carpenter 2011-02-23 15:09 ` Martyn Welch 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2011-02-23 12:17 UTC (permalink / raw) To: Martyn Welch; +Cc: Manohar Vanga, devel, cota, linux-kernel On Wed, Feb 23, 2011 at 09:19:06AM +0000, Martyn Welch wrote: > On 22/02/11 19:36, Manohar Vanga wrote: > > Remove unreachable code from vme_register_bridge > > > > Signed-off-by: Manohar Vanga <manohar.vanga@cern.ch> Please always CC the list. It's going to be Greg to commit this code so CC Greg as well. ./scripts/get_maintainer.pl patch.diff (Although you should generally remove Tejun from the CC list.) > > Yeah - that's there from development. If the function needed to be extended, > that's the next part of the error path. Don't do that. Kernel style is austere. Anything that isn't needed here and now should be removed. > > --- a/drivers/staging/vme/vme.c > > +++ b/drivers/staging/vme/vme.c > > @@ -1363,7 +1363,6 @@ int vme_register_bridge(struct vme_bridge *bridge) > > > > return retval; > > > > - i = VME_SLOTS_MAX; > > err_reg: > > while (i > -1) { > > dev = &bridge->dev[i]; There are two problems with this loop. 1) It unregisters the device which failed to register. 2) It is a forever loop. It should be: while (--i >= 0) { dev = &bridge->dev[i]; Can you fix that up as well and resend. regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Staging: vme: remove unreachable code 2011-02-23 12:17 ` Dan Carpenter @ 2011-02-23 15:09 ` Martyn Welch 2011-02-23 15:16 ` Manohar Vanga 0 siblings, 1 reply; 6+ messages in thread From: Martyn Welch @ 2011-02-23 15:09 UTC (permalink / raw) To: Dan Carpenter, Manohar Vanga, devel, cota, linux-kernel On 23/02/11 12:17, Dan Carpenter wrote: > On Wed, Feb 23, 2011 at 09:19:06AM +0000, Martyn Welch wrote: >> On 22/02/11 19:36, Manohar Vanga wrote: >>> Remove unreachable code from vme_register_bridge >>> >>> Signed-off-by: Manohar Vanga <manohar.vanga@cern.ch> > > Please always CC the list. It's going to be Greg to commit this > code so CC Greg as well. > > ./scripts/get_maintainer.pl patch.diff > > (Although you should generally remove Tejun from the CC list.) > >> >> Yeah - that's there from development. If the function needed to be extended, >> that's the next part of the error path. > > Don't do that. Kernel style is austere. Anything that isn't needed > here and now should be removed. > >>> --- a/drivers/staging/vme/vme.c >>> +++ b/drivers/staging/vme/vme.c >>> @@ -1363,7 +1363,6 @@ int vme_register_bridge(struct vme_bridge *bridge) >>> >>> return retval; >>> >>> - i = VME_SLOTS_MAX; >>> err_reg: >>> while (i > -1) { >>> dev = &bridge->dev[i]; > > There are two problems with this loop. 1) It unregisters the device > which failed to register. 2) It is a forever loop. > Even if "i" is a signed int? > It should be: > while (--i >= 0) { > dev = &bridge->dev[i]; > > Can you fix that up as well and resend. > > regards, > dan carpenter -- Martyn Welch (Principal Software Engineer) | Registered in England and GE Intelligent Platforms | Wales (3828642) at 100 T +44(0)127322748 | Barbirolli Square, Manchester, E martyn.welch@ge.com | M2 3AB VAT:GB 927559189 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Staging: vme: remove unreachable code 2011-02-23 15:09 ` Martyn Welch @ 2011-02-23 15:16 ` Manohar Vanga 2011-02-23 15:19 ` Martyn Welch 0 siblings, 1 reply; 6+ messages in thread From: Manohar Vanga @ 2011-02-23 15:16 UTC (permalink / raw) To: Martyn Welch; +Cc: Dan Carpenter, devel, cota, linux-kernel > > There are two problems with this loop. 1) It unregisters the device > > which failed to register. 2) It is a forever loop. > > > > Even if "i" is a signed int? The infinite loop is because "i" is not decremented anywhere in the loop. Not because of signedness. Thanks, Manohar Vanga ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Staging: vme: remove unreachable code 2011-02-23 15:16 ` Manohar Vanga @ 2011-02-23 15:19 ` Martyn Welch 0 siblings, 0 replies; 6+ messages in thread From: Martyn Welch @ 2011-02-23 15:19 UTC (permalink / raw) To: Manohar Vanga; +Cc: Dan Carpenter, devel, cota, linux-kernel On 23/02/11 15:16, Manohar Vanga wrote: >>> There are two problems with this loop. 1) It unregisters the device >>> which failed to register. 2) It is a forever loop. >>> >> >> Even if "i" is a signed int? > > The infinite loop is because "i" is not decremented anywhere in the loop. Not > because of signedness. > Ah, yeah, that would help. Martyn > Thanks, > Manohar Vanga -- Martyn Welch (Principal Software Engineer) | Registered in England and GE Intelligent Platforms | Wales (3828642) at 100 T +44(0)127322748 | Barbirolli Square, Manchester, E martyn.welch@ge.com | M2 3AB VAT:GB 927559189 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <1298403376-28352-3-git-send-email-manohar.vanga@cern.ch>]
* Re: [PATCH 2/2] Staging: vme: fix potential infinite loop [not found] ` <1298403376-28352-3-git-send-email-manohar.vanga@cern.ch> @ 2011-02-23 9:24 ` Manohar Vanga 0 siblings, 0 replies; 6+ messages in thread From: Manohar Vanga @ 2011-02-23 9:24 UTC (permalink / raw) To: martyn.welch; +Cc: cota, devel, linux-kernel On Tue, Feb 22, 2011 at 08:36:16PM +0100, Manohar Vanga wrote: > Fix a bug in vme_register_bridge that resulted in an infinite loop if > device_register failed. > > Signed-off-by: Manohar Vanga <manohar.vanga@cern.ch> > --- > drivers/staging/vme/vme.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/drivers/staging/vme/vme.c b/drivers/staging/vme/vme.c > index 88bf455..7eb4568 100644 > --- a/drivers/staging/vme/vme.c > +++ b/drivers/staging/vme/vme.c > @@ -1364,9 +1364,14 @@ int vme_register_bridge(struct vme_bridge *bridge) > return retval; > > err_reg: > + /* > + * Ignore the failed driver and unregister the rest > + */ > + i-- Ugh. Missing semicolon here. I'll resend the corrected patch. Thanks, Manohar Vanga ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-02-23 15:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1298403376-28352-1-git-send-email-manohar.vanga@cern.ch>
[not found] ` <1298403376-28352-2-git-send-email-manohar.vanga@cern.ch>
2011-02-23 9:19 ` [PATCH 1/2] Staging: vme: remove unreachable code Martyn Welch
2011-02-23 12:17 ` Dan Carpenter
2011-02-23 15:09 ` Martyn Welch
2011-02-23 15:16 ` Manohar Vanga
2011-02-23 15:19 ` Martyn Welch
[not found] ` <1298403376-28352-3-git-send-email-manohar.vanga@cern.ch>
2011-02-23 9:24 ` [PATCH 2/2] Staging: vme: fix potential infinite loop Manohar Vanga
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox