Re: [patch] staging: bcm2835-camera: free first element in array
On Wed, Feb 15, 2017 at 01:47:55PM +0100, walter harms wrote: > > > Am 15.02.2017 13:25, schrieb Dan Carpenter: > > We should free gdev[0] so the > should be >=. > > > > Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera > > driver.") > > Signed-off-by: Dan Carpenter> > > > diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > > b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > > index ca15a698e018..9bcd8e546a14 100644 > > --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > > +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > > @@ -1998,7 +1998,7 @@ static int __init bm2835_mmal_init(void) > > free_dev: > > kfree(dev); > > > > - for ( ; camera > 0; camera--) { > > + for ( ; camera >= 0; camera--) { > > bcm2835_cleanup_instance(gdev[camera]); > > gdev[camera] = NULL; > > } > > since we already know that programmers are bad in counting backwards ... > > is is possible to change that into std. loop like: > > for(i=0, i< camera; i++ { > bcm2835_cleanup_instance(gdev[i]); > gdev[i] = NULL; > } > > this is way a much more common pattern. Hm... My patch is buggy. It frees the wong thing on the first iteration through the loop. I'll resend. regards, dan carpenter
Re: [patch] staging: bcm2835-camera: free first element in array
Am 15.02.2017 13:25, schrieb Dan Carpenter: > We should free gdev[0] so the > should be >=. > > Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera > driver.") > Signed-off-by: Dan Carpenter> > diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > index ca15a698e018..9bcd8e546a14 100644 > --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c > @@ -1998,7 +1998,7 @@ static int __init bm2835_mmal_init(void) > free_dev: > kfree(dev); > > - for ( ; camera > 0; camera--) { > + for ( ; camera >= 0; camera--) { > bcm2835_cleanup_instance(gdev[camera]); > gdev[camera] = NULL; > } since we already know that programmers are bad in counting backwards ... is is possible to change that into std. loop like: for(i=0, i< camera; i++ { bcm2835_cleanup_instance(gdev[i]); gdev[i] = NULL; } this is way a much more common pattern. just my 2 cents, re, wh
[patch] staging: bcm2835-camera: free first element in array
We should free gdev[0] so the > should be >=. Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera driver.") Signed-off-by: Dan Carpenterdiff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c index ca15a698e018..9bcd8e546a14 100644 --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c @@ -1998,7 +1998,7 @@ static int __init bm2835_mmal_init(void) free_dev: kfree(dev); - for ( ; camera > 0; camera--) { + for ( ; camera >= 0; camera--) { bcm2835_cleanup_instance(gdev[camera]); gdev[camera] = NULL; }