Re: [Intel-gfx] [PATCH v2] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-24 08:42:06)
> 
> On 23/11/2017 21:17, Chris Wilson wrote:
> > We pretend the PMU config id is a pointer value when encoding it into
> > the device parameters for presentation via sysfs. This requires casting
> > of an unsigned long into and out of the pointer member, which annoys
> > smatch:
> > 
> > drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 
> > to %lx specifier is cast from pointer
> > 
> > Instead of abusing a generic dev_ext_attribute, define our own typesafe
> > attributes.
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Tvrtko Ursulin 
> > Cc: Michal Wajdeczko 
[snip]
> Reviewed-by: Tvrtko Ursulin 

Thanks, it's such a trivial warning that I wasn't such if it was worth
the effort. But we are close to being smatch clean, and in the past it
has caught genuine bugs that gcc/sparse miss so I think it is a valuable
tool, and so worth the effort to get and stay clean.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Tvrtko Ursulin]

On 23/11/2017 21:17, Chris Wilson wrote:

We pretend the PMU config id is a pointer value when encoding it into
the device parameters for presentation via sysfs. This requires casting
of an unsigned long into and out of the pointer member, which annoys
smatch:

drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to 
%lx specifier is cast from pointer

Instead of abusing a generic dev_ext_attribute, define our own typesafe
attributes.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Michal Wajdeczko 
---
  drivers/gpu/drm/i915/i915_pmu.c | 30 --
  1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 6a42e7f7967d..5170a46893d7 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -650,19 +650,24 @@ static int i915_pmu_event_event_idx(struct perf_event 
*event)
return 0;
  }
  
+struct i915_str_attribute {

+   struct device_attribute attr;
+   const char *str;
+};
+
  static ssize_t i915_pmu_format_show(struct device *dev,
struct device_attribute *attr, char *buf)
  {
-   struct dev_ext_attribute *eattr;
+   struct i915_str_attribute *eattr;
  
-	eattr = container_of(attr, struct dev_ext_attribute, attr);

-   return sprintf(buf, "%s\n", (char *)eattr->var);
+   eattr = container_of(attr, struct i915_str_attribute, attr);
+   return sprintf(buf, "%s\n", eattr->str);
  }
  
  #define I915_PMU_FORMAT_ATTR(_name, _config) \

-   (&((struct dev_ext_attribute[]) { \
+   (&((struct i915_str_attribute[]) { \
{ .attr = __ATTR(_name, 0444, i915_pmu_format_show, NULL), \
- .var = (void *)_config, } \
+ .str = _config, } \
})[0].attr.attr)
  
  static struct attribute *i915_pmu_format_attrs[] = {

@@ -675,19 +680,24 @@ static const struct attribute_group 
i915_pmu_format_attr_group = {
.attrs = i915_pmu_format_attrs,
  };
  
+struct i915_ext_attribute {

+   struct device_attribute attr;
+   unsigned long val;
+};
+
  static ssize_t i915_pmu_event_show(struct device *dev,
   struct device_attribute *attr, char *buf)
  {
-   struct dev_ext_attribute *eattr;
+   struct i915_ext_attribute *eattr;
  
-	eattr = container_of(attr, struct dev_ext_attribute, attr);

-   return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
+   eattr = container_of(attr, struct i915_ext_attribute, attr);
+   return sprintf(buf, "config=0x%lx\n", eattr->val);
  }
  
  #define I915_EVENT_ATTR(_name, _config) \

-   (&((struct dev_ext_attribute[]) { \
+   (&((struct i915_ext_attribute[]) { \
{ .attr = __ATTR(_name, 0444, i915_pmu_event_show, NULL), \
- .var = (void *)_config, } \
+ .val = _config, } \
})[0].attr.attr)
  
  #define I915_EVENT_STR(_name, _str) \




Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915/pmu: Hide the (unsigned long)ptr cast

[Chris Wilson]

We pretend the PMU config id is a pointer value when encoding it into
the device parameters for presentation via sysfs. This requires casting
of an unsigned long into and out of the pointer member, which annoys
smatch:

drivers/gpu/drm/i915/i915_pmu.c:684 i915_pmu_event_show() warn: argument 3 to 
%lx specifier is cast from pointer

Instead of abusing a generic dev_ext_attribute, define our own typesafe
attributes.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Michal Wajdeczko 
---
 drivers/gpu/drm/i915/i915_pmu.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 6a42e7f7967d..5170a46893d7 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -650,19 +650,24 @@ static int i915_pmu_event_event_idx(struct perf_event 
*event)
return 0;
 }
 
+struct i915_str_attribute {
+   struct device_attribute attr;
+   const char *str;
+};
+
 static ssize_t i915_pmu_format_show(struct device *dev,
struct device_attribute *attr, char *buf)
 {
-   struct dev_ext_attribute *eattr;
+   struct i915_str_attribute *eattr;
 
-   eattr = container_of(attr, struct dev_ext_attribute, attr);
-   return sprintf(buf, "%s\n", (char *)eattr->var);
+   eattr = container_of(attr, struct i915_str_attribute, attr);
+   return sprintf(buf, "%s\n", eattr->str);
 }
 
 #define I915_PMU_FORMAT_ATTR(_name, _config) \
-   (&((struct dev_ext_attribute[]) { \
+   (&((struct i915_str_attribute[]) { \
{ .attr = __ATTR(_name, 0444, i915_pmu_format_show, NULL), \
- .var = (void *)_config, } \
+ .str = _config, } \
})[0].attr.attr)
 
 static struct attribute *i915_pmu_format_attrs[] = {
@@ -675,19 +680,24 @@ static const struct attribute_group 
i915_pmu_format_attr_group = {
.attrs = i915_pmu_format_attrs,
 };
 
+struct i915_ext_attribute {
+   struct device_attribute attr;
+   unsigned long val;
+};
+
 static ssize_t i915_pmu_event_show(struct device *dev,
   struct device_attribute *attr, char *buf)
 {
-   struct dev_ext_attribute *eattr;
+   struct i915_ext_attribute *eattr;
 
-   eattr = container_of(attr, struct dev_ext_attribute, attr);
-   return sprintf(buf, "config=0x%lx\n", (unsigned long)eattr->var);
+   eattr = container_of(attr, struct i915_ext_attribute, attr);
+   return sprintf(buf, "config=0x%lx\n", eattr->val);
 }
 
 #define I915_EVENT_ATTR(_name, _config) \
-   (&((struct dev_ext_attribute[]) { \
+   (&((struct i915_ext_attribute[]) { \
{ .attr = __ATTR(_name, 0444, i915_pmu_event_show, NULL), \
- .var = (void *)_config, } \
+ .val = _config, } \
})[0].attr.attr)
 
 #define I915_EVENT_STR(_name, _str) \
-- 
2.15.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx