Hi! > On Tue, Jul 25, 2017 at 02:30:31PM +0200, Johan Hovold wrote: > > [ +CC: Rui and Greg ] > > Thanks Johan. I only got this because of you.
> > > return ret; > > > } > > > > And while it's fine to take this through linux-media, it would still be > > good to keep the maintainers on CC. > > Sakari, if you could resend the all series to the right lists and > maintainers for proper review that would be great. > > I did not get 0/2 and 2/2 patches. 0/2 and 2/2 were unrelated to the memory leak, IIRC. Let me google it for you... https://www.mail-archive.com/linux-media@vger.kernel.org/msg115840.html This is memory leak and the driver is in staging. Acked-by or fixing it yourself would be appropriate response, asking for resending of the series... not quite so. Best regards, Pavel > > On Tue, Jul 18, 2017 at 09:41:06PM +0300, Sakari Ailus wrote: > > > Memory for struct v4l2_flash_config is allocated in > > > gb_lights_light_v4l2_register() for no gain and yet the allocated memory > > > is > > > leaked; the struct isn't used outside the function. Fix this. > > > > > > Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com> > > > --- > > > drivers/staging/greybus/light.c | 17 ++++++----------- > > > 1 file changed, 6 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/staging/greybus/light.c > > > b/drivers/staging/greybus/light.c > > > index 129ceed39829..b25c117ec41a 100644 > > > --- a/drivers/staging/greybus/light.c > > > +++ b/drivers/staging/greybus/light.c > > > @@ -534,25 +534,21 @@ static int gb_lights_light_v4l2_register(struct > > > gb_light *light) > > > { > > > struct gb_connection *connection = get_conn_from_light(light); > > > struct device *dev = &connection->bundle->dev; > > > - struct v4l2_flash_config *sd_cfg; > > > + struct v4l2_flash_config sd_cfg = { 0 }; > > > struct led_classdev_flash *fled; > > > struct led_classdev *iled = NULL; > > > struct gb_channel *channel_torch, *channel_ind, *channel_flash; > > > int ret = 0; > > > > > > - sd_cfg = kcalloc(1, sizeof(*sd_cfg), GFP_KERNEL); > > > - if (!sd_cfg) > > > - return -ENOMEM; > > > - > > > channel_torch = get_channel_from_mode(light, GB_CHANNEL_MODE_TORCH); > > > if (channel_torch) > > > __gb_lights_channel_v4l2_config(&channel_torch->intensity_uA, > > > - &sd_cfg->torch_intensity); > > > + &sd_cfg.torch_intensity); > > > > > > channel_ind = get_channel_from_mode(light, GB_CHANNEL_MODE_INDICATOR); > > > if (channel_ind) { > > > __gb_lights_channel_v4l2_config(&channel_ind->intensity_uA, > > > - &sd_cfg->indicator_intensity); > > > + &sd_cfg.indicator_intensity); > > > iled = &channel_ind->fled.led_cdev; > > > } > > > > > > @@ -561,17 +557,17 @@ static int gb_lights_light_v4l2_register(struct > > > gb_light *light) > > > > > > fled = &channel_flash->fled; > > > > > > - snprintf(sd_cfg->dev_name, sizeof(sd_cfg->dev_name), "%s", light->name); > > > + snprintf(sd_cfg.dev_name, sizeof(sd_cfg.dev_name), "%s", light->name); > > > > > > /* Set the possible values to faults, in our case all faults */ > > > - sd_cfg->flash_faults = LED_FAULT_OVER_VOLTAGE | LED_FAULT_TIMEOUT | > > > + sd_cfg.flash_faults = LED_FAULT_OVER_VOLTAGE | LED_FAULT_TIMEOUT | > > > LED_FAULT_OVER_TEMPERATURE | LED_FAULT_SHORT_CIRCUIT | > > > LED_FAULT_OVER_CURRENT | LED_FAULT_INDICATOR | > > > LED_FAULT_UNDER_VOLTAGE | LED_FAULT_INPUT_VOLTAGE | > > > LED_FAULT_LED_OVER_TEMPERATURE; > > > > > > light->v4l2_flash = v4l2_flash_init(dev, NULL, fled, iled, > > > - &v4l2_flash_ops, sd_cfg); > > > + &v4l2_flash_ops, &sd_cfg); > > > if (IS_ERR_OR_NULL(light->v4l2_flash)) { > > > ret = PTR_ERR(light->v4l2_flash); > > > goto out_free; > > > @@ -580,7 +576,6 @@ static int gb_lights_light_v4l2_register(struct > > > gb_light *light) > > > return ret; > > > > > > out_free: > > > - kfree(sd_cfg); > > > > This looks a bit lazy, even if I just noticed that you repurpose this > > error label (without renaming it) in you second patch. > > > > -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
