re: drm/amd/powerplay: add dpm force multiple levels on cz/tonga/fiji/polaris (v2)

re: drm/amd/powerplay: add dpm force multiple levels on cz/tonga/fiji/polaris (v2)

[Dan Carpenter]

Hello Eric Huang,

The patch 5632708f4452: "drm/amd/powerplay: add dpm force multiple
levels on cz/tonga/fiji/polaris (v2)" from Apr 12, 2016, leads to the
following static checker warning:

	drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:368 amdgpu_set_pp_dpm_sclk()
	warn: why is the last element skipped?

drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
   356  static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
   357                  struct device_attribute *attr,
   358                  const char *buf,
   359                  size_t count)
   360  {
   361          struct drm_device *ddev = dev_get_drvdata(dev);
   362          struct amdgpu_device *adev = ddev->dev_private;
   363          int ret;
   364          long level;
   365          uint32_t i, mask = 0;
   366          char sub_str[2];
   367  
   368          for (i = 0; i < strlen(buf) - 1; i++) {

strlen() returns the number of characters not counting the NUL
terminator.  I guess this assumes that the last character is a newline?
Could we check for that instead?

   369                  sub_str[0] = *(buf + i);
   370                  sub_str[1] = '\0';
   371                  ret = kstrtol(sub_str, 0, &level);
   372  
   373                  if (ret) {
   374                          count = -EINVAL;
   375                          goto fail;
   376                  }
   377                  mask |= 1 << level;
   378          }
   379  
   380          if (adev->pp_enabled)
   381                  amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
   382  fail:
   383          return count;
   384  }

There are a couple other:
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:412 amdgpu_set_pp_dpm_mclk() warn: why is the last element skipped?
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:456 amdgpu_set_pp_dpm_pcie() warn: why is the last element skipped?

regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: drm/amd/powerplay: add dpm force multiple levels on cz/tonga/fiji/polaris (v2)

[Eric Huang]

Hi Dan,

It is for avoiding the last char of newline. I will change it.

Thanks,
Eric

On 16-05-10 06:51 AM, Dan Carpenter wrote:
> Hello Eric Huang,
>
> The patch 5632708f4452: "drm/amd/powerplay: add dpm force multiple
> levels on cz/tonga/fiji/polaris (v2)" from Apr 12, 2016, leads to the
> following static checker warning:
>
> 	drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:368 amdgpu_set_pp_dpm_sclk()
> 	warn: why is the last element skipped?
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>     356  static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
>     357                  struct device_attribute *attr,
>     358                  const char *buf,
>     359                  size_t count)
>     360  {
>     361          struct drm_device *ddev = dev_get_drvdata(dev);
>     362          struct amdgpu_device *adev = ddev->dev_private;
>     363          int ret;
>     364          long level;
>     365          uint32_t i, mask = 0;
>     366          char sub_str[2];
>     367
>     368          for (i = 0; i < strlen(buf) - 1; i++) {
>
> strlen() returns the number of characters not counting the NUL
> terminator.  I guess this assumes that the last character is a newline?
> Could we check for that instead?
>
>     369                  sub_str[0] = *(buf + i);
>     370                  sub_str[1] = '\0';
>     371                  ret = kstrtol(sub_str, 0, &level);
>     372
>     373                  if (ret) {
>     374                          count = -EINVAL;
>     375                          goto fail;
>     376                  }
>     377                  mask |= 1 << level;
>     378          }
>     379
>     380          if (adev->pp_enabled)
>     381                  amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
>     382  fail:
>     383          return count;
>     384  }
>
> There are a couple other:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:412 amdgpu_set_pp_dpm_mclk() warn: why is the last element skipped?
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c:456 amdgpu_set_pp_dpm_pcie() warn: why is the last element skipped?
>
> regards,
> dan carpenter

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel