On 09/08/17 13:15, Sakari Ailus wrote: > From: Rui Miguel Silva <rmfrfs@xxxxxxxxx> > > We are allocating memory for the v4l2 flash configuration structure and > leak it in the normal path. Just use the stack for this as we do not > use it outside of this function. I'm not sure why this is part of this patch series. This is a greybus bug fix, right? And independent from the other two patches. > > Fixes: 2870b52bae4c ("greybus: lights: add lights implementation") > Reported-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > Signed-off-by: Rui Miguel Silva <rmfrfs@xxxxxxxxx> > Reviewed-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx> > --- > drivers/staging/greybus/light.c | 29 +++++++++-------------------- > 1 file changed, 9 insertions(+), 20 deletions(-) > > diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c > index 129ceed39829..0771db418f84 100644 > --- a/drivers/staging/greybus/light.c > +++ b/drivers/staging/greybus/light.c > @@ -534,25 +534,20 @@ 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} }; Just use '= {};' > 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,27 +556,21 @@ 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); > - if (IS_ERR_OR_NULL(light->v4l2_flash)) { > - ret = PTR_ERR(light->v4l2_flash); > - goto out_free; > - } > + &v4l2_flash_ops, &sd_cfg); > + if (IS_ERR_OR_NULL(light->v4l2_flash)) Just IS_ERR since v4l2_flash_init() never returns NULL. > + return PTR_ERR(light->v4l2_flash); > > - return ret; > - > -out_free: > - kfree(sd_cfg); > - return ret; > + return 0; > } > > static void gb_lights_light_v4l2_unregister(struct gb_light *light) > After those two changes: Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> Regards, Hans