[ +CC: Rui and Greg ]
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.ailus@xxxxxxxxxxxxxxx>
> ---
> 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.
> return ret;
> }
And while it's fine to take this through linux-media, it would still be
good to keep the maintainers on CC.
Thanks,
Johan
