re: ACPI / video: Run _BCL before deciding registering backlight

re: ACPI / video: Run _BCL before deciding registering backlight

[Dan Carpenter]

Hello Aaron Lu,

The patch dce4ec2e452f: "ACPI / video: Run _BCL before deciding
registering backlight" from Oct 28, 2014, leads to the following
static checker warning:

	drivers/acpi/acpi_video.c:1659 acpi_video_run_bcl_for_osi()
	warn: calling kfree() when 'levels' is always NULL.

drivers/acpi/acpi_video.c
  1651  static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
  1652  {
  1653          struct acpi_video_device *dev;
  1654          union acpi_object *levels;
  1655  
  1656          mutex_lock(&video->device_list_lock);
  1657          list_for_each_entry(dev, &video->video_device_list, entry) {
  1658                  if (!acpi_video_device_lcd_query_levels(dev, &levels))
  1659                          kfree(levels);

kfree(NULL) is a no-op.  We could just remove the if statement here.

		list_for_each_entry(dev, &video->video_device_list, entry) {
			acpi_video_device_lcd_query_levels(dev, &levels);
		}

But then it's not clear to me the point of this loop is?

  1660          }
  1661          mutex_unlock(&video->device_list_lock);
  1662  }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in

Re: ACPI / video: Run _BCL before deciding registering backlight

[Dan Carpenter]

This static checker warning was totally wrong.  Sorry for the noise.

regards,
dan carpente

On Sat, Jun 20, 2015 at 03:26:39PM +0300, Dan Carpenter wrote:
> Hello Aaron Lu,
> 
> The patch dce4ec2e452f: "ACPI / video: Run _BCL before deciding
> registering backlight" from Oct 28, 2014, leads to the following
> static checker warning:
> 
> 	drivers/acpi/acpi_video.c:1659 acpi_video_run_bcl_for_osi()
> 	warn: calling kfree() when 'levels' is always NULL.
> 
> drivers/acpi/acpi_video.c
>   1651  static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
>   1652  {
>   1653          struct acpi_video_device *dev;
>   1654          union acpi_object *levels;
>   1655  
>   1656          mutex_lock(&video->device_list_lock);
>   1657          list_for_each_entry(dev, &video->video_device_list, entry) {
>   1658                  if (!acpi_video_device_lcd_query_levels(dev, &levels))
>   1659                          kfree(levels);
> 
> kfree(NULL) is a no-op.  We could just remove the if statement here.
> 
> 		list_for_each_entry(dev, &video->video_device_list, entry) {
> 			acpi_video_device_lcd_query_levels(dev, &levels);
> 		}
> 
> But then it's not clear to me the point of this loop is?
> 
>   1660          }
>   1661          mutex_unlock(&video->device_list_lock);
>   1662  }
> 
> regards,
> dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in