[Intel-gfx] [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-02 Thread Dave Airlie
On Thu, Dec 2, 2010 at 5:38 AM, Andrew Morton  
wrote:
> On Wed, 1 Dec 2010 17:54:18 +0100
> Julien Cristau  wrote:
>
>> On Wed, Dec ?1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
>>
>> > For headers that get exported to userland and make use of u32 style
>> > type names, it is advised to include linux/types.h.
>> >
>> > This fixes 5 headers_check warnings.
>> >
>> How many times does this need to be NAKed?
>
> Until someone gets a clue and puts comments in there explaining this?

how about someone fixing the dumb scripts to understand that C header
includes aren't single level.

Like 10 people have posted this patch and not one has come back with a
fix for the app after I pointed it out, like really if people think
they can write C good enough to send kernel patches,
maybe they could put some more effort in and actually fix a real problem.

We should start hashing signed-off-by's so people can't get any glory from them.

Dave.


Re: [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-02 Thread Alexander Shishkin
On Wed, Dec 01, 2010 at 05:54:18PM +0100, Julien Cristau wrote:
 On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
 
  For headers that get exported to userland and make use of u32 style
  type names, it is advised to include linux/types.h.
  
  This fixes 5 headers_check warnings.
  
 How many times does this need to be NAKed?  These headers are shared
 with the BSDs, and they include drm.h which has the linux/types.h
 include on linux already.

One of the rules of including files that comes to mind is that one should
never rely on stuff being included from other headers but always explicitly
include those which are needed.
But if compatibility with other OS kernels is a valid reason to disregard
common coding practices, maybe at least it deserves a comment in those files?

But I really couldn't care less about these headers, so this is totally up
to you.

Regards,
--
Alex
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Alexander Shishkin
On Wed, Dec 01, 2010 at 05:54:18PM +0100, Julien Cristau wrote:
> On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
> 
> > For headers that get exported to userland and make use of u32 style
> > type names, it is advised to include linux/types.h.
> > 
> > This fixes 5 headers_check warnings.
> > 
> How many times does this need to be NAKed?  These headers are shared
> with the BSDs, and they include drm.h which has the linux/types.h
> include on linux already.

One of the rules of including files that comes to mind is that one should
never rely on stuff being included from other headers but always explicitly
include those which are needed.
But if compatibility with other OS kernels is a valid reason to disregard
common coding practices, maybe at least it deserves a comment in those files?

But I really couldn't care less about these headers, so this is totally up
to you.

Regards,
--
Alex


[PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Julien Cristau
On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:

> For headers that get exported to userland and make use of u32 style
> type names, it is advised to include linux/types.h.
> 
> This fixes 5 headers_check warnings.
> 
How many times does this need to be NAKed?  These headers are shared
with the BSDs, and they include drm.h which has the linux/types.h
include on linux already.

Cheers,
Julien


[PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Alexander Shishkin
For headers that get exported to userland and make use of u32 style
type names, it is advised to include linux/types.h.

This fixes 5 headers_check warnings.

Signed-off-by: Alexander Shishkin 
CC: Andrew Morton 
CC: David Airlie 
CC: Chris Wilson 
CC: dri-devel at lists.freedesktop.org
CC: linux-kernel at vger.kernel.org
CC: intel-gfx at lists.freedesktop.org
---
 include/drm/drm_mode.h   |2 ++
 include/drm/i915_drm.h   |1 +
 include/drm/mga_drm.h|1 +
 include/drm/radeon_drm.h |1 +
 include/drm/via_drm.h|1 +
 5 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 0fc7397..1678d7b 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -27,6 +27,8 @@
 #ifndef _DRM_MODE_H
 #define _DRM_MODE_H

+#include 
+
 #define DRM_DISPLAY_INFO_LEN   32
 #define DRM_CONNECTOR_NAME_LEN 32
 #define DRM_DISPLAY_MODE_LEN   32
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 8c641bed..c07c043 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -27,6 +27,7 @@
 #ifndef _I915_DRM_H_
 #define _I915_DRM_H_

+#include 
 #include "drm.h"

 /* Please note that modifications to all structs defined here are
diff --git a/include/drm/mga_drm.h b/include/drm/mga_drm.h
index c16097f..1107097 100644
--- a/include/drm/mga_drm.h
+++ b/include/drm/mga_drm.h
@@ -35,6 +35,7 @@
 #ifndef __MGA_DRM_H__
 #define __MGA_DRM_H__

+#include 
 #include "drm.h"

 /* WARNING: If you change any of these defines, make sure to change the
diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
index 10f8b53..78d9344 100644
--- a/include/drm/radeon_drm.h
+++ b/include/drm/radeon_drm.h
@@ -33,6 +33,7 @@
 #ifndef __RADEON_DRM_H__
 #define __RADEON_DRM_H__

+#include 
 #include "drm.h"

 /* WARNING: If you change any of these defines, make sure to change the
diff --git a/include/drm/via_drm.h b/include/drm/via_drm.h
index fd11a5b..23880b0 100644
--- a/include/drm/via_drm.h
+++ b/include/drm/via_drm.h
@@ -24,6 +24,7 @@
 #ifndef _VIA_DRM_H_
 #define _VIA_DRM_H_

+#include 
 #include "drm.h"

 /* WARNING: These defines must be the same as what the Xserver uses.
-- 
1.7.2.1.45.gb66c2



[PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Kristian Høgsberg
On Wed, Dec 1, 2010 at 11:54 AM, Julien Cristau  wrote:
> On Wed, Dec ?1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
>
>> For headers that get exported to userland and make use of u32 style
>> type names, it is advised to include linux/types.h.
>>
>> This fixes 5 headers_check warnings.
>>
> How many times does this need to be NAKed? ?These headers are shared
> with the BSDs, and they include drm.h which has the linux/types.h
> include on linux already.

I'll NAK it too, if that helps.

Kristian


[Intel-gfx] [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Randy Dunlap
On Thu, 2 Dec 2010 05:55:59 +1000 Dave Airlie wrote:

> On Thu, Dec 2, 2010 at 5:38 AM, Andrew Morton  
> wrote:
> > On Wed, 1 Dec 2010 17:54:18 +0100
> > Julien Cristau  wrote:
> >
> >> On Wed, Dec ?1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
> >>
> >> > For headers that get exported to userland and make use of u32 style
> >> > type names, it is advised to include linux/types.h.
> >> >
> >> > This fixes 5 headers_check warnings.
> >> >
> >> How many times does this need to be NAKed?
> >
> > Until someone gets a clue and puts comments in there explaining this?
> 
> how about someone fixing the dumb scripts to understand that C header
> includes aren't single level.

Still, drm is an exception here, so it needs to be documented as such.


> Like 10 people have posted this patch and not one has come back with a
> fix for the app after I pointed it out, like really if people think
> they can write C good enough to send kernel patches,
> maybe they could put some more effort in and actually fix a real problem.
> 
> We should start hashing signed-off-by's so people can't get any glory from 
> them.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***


[PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Andrew Morton
On Wed, 1 Dec 2010 17:54:18 +0100
Julien Cristau  wrote:

> On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
> 
> > For headers that get exported to userland and make use of u32 style
> > type names, it is advised to include linux/types.h.
> > 
> > This fixes 5 headers_check warnings.
> > 
> How many times does this need to be NAKed?

Until someone gets a clue and puts comments in there explaining this?


[PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Alexander Shishkin
For headers that get exported to userland and make use of u32 style
type names, it is advised to include linux/types.h.

This fixes 5 headers_check warnings.

Signed-off-by: Alexander Shishkin virtu...@slind.org
CC: Andrew Morton a...@linux-foundation.org
CC: David Airlie airl...@linux.ie
CC: Chris Wilson ch...@chris-wilson.co.uk
CC: dri-devel@lists.freedesktop.org
CC: linux-ker...@vger.kernel.org
CC: intel-...@lists.freedesktop.org
---
 include/drm/drm_mode.h   |2 ++
 include/drm/i915_drm.h   |1 +
 include/drm/mga_drm.h|1 +
 include/drm/radeon_drm.h |1 +
 include/drm/via_drm.h|1 +
 5 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 0fc7397..1678d7b 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -27,6 +27,8 @@
 #ifndef _DRM_MODE_H
 #define _DRM_MODE_H
 
+#include linux/types.h
+
 #define DRM_DISPLAY_INFO_LEN   32
 #define DRM_CONNECTOR_NAME_LEN 32
 #define DRM_DISPLAY_MODE_LEN   32
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 8c641bed..c07c043 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -27,6 +27,7 @@
 #ifndef _I915_DRM_H_
 #define _I915_DRM_H_
 
+#include linux/types.h
 #include drm.h
 
 /* Please note that modifications to all structs defined here are
diff --git a/include/drm/mga_drm.h b/include/drm/mga_drm.h
index c16097f..1107097 100644
--- a/include/drm/mga_drm.h
+++ b/include/drm/mga_drm.h
@@ -35,6 +35,7 @@
 #ifndef __MGA_DRM_H__
 #define __MGA_DRM_H__
 
+#include linux/types.h
 #include drm.h
 
 /* WARNING: If you change any of these defines, make sure to change the
diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
index 10f8b53..78d9344 100644
--- a/include/drm/radeon_drm.h
+++ b/include/drm/radeon_drm.h
@@ -33,6 +33,7 @@
 #ifndef __RADEON_DRM_H__
 #define __RADEON_DRM_H__
 
+#include linux/types.h
 #include drm.h
 
 /* WARNING: If you change any of these defines, make sure to change the
diff --git a/include/drm/via_drm.h b/include/drm/via_drm.h
index fd11a5b..23880b0 100644
--- a/include/drm/via_drm.h
+++ b/include/drm/via_drm.h
@@ -24,6 +24,7 @@
 #ifndef _VIA_DRM_H_
 #define _VIA_DRM_H_
 
+#include linux/types.h
 #include drm.h
 
 /* WARNING: These defines must be the same as what the Xserver uses.
-- 
1.7.2.1.45.gb66c2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Julien Cristau
On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:

 For headers that get exported to userland and make use of u32 style
 type names, it is advised to include linux/types.h.
 
 This fixes 5 headers_check warnings.
 
How many times does this need to be NAKed?  These headers are shared
with the BSDs, and they include drm.h which has the linux/types.h
include on linux already.

Cheers,
Julien
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Kristian Høgsberg
On Wed, Dec 1, 2010 at 11:54 AM, Julien Cristau jcris...@debian.org wrote:
 On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:

 For headers that get exported to userland and make use of u32 style
 type names, it is advised to include linux/types.h.

 This fixes 5 headers_check warnings.

 How many times does this need to be NAKed?  These headers are shared
 with the BSDs, and they include drm.h which has the linux/types.h
 include on linux already.

I'll NAK it too, if that helps.

Kristian
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Andrew Morton
On Wed, 1 Dec 2010 17:54:18 +0100
Julien Cristau jcris...@debian.org wrote:

 On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
 
  For headers that get exported to userland and make use of u32 style
  type names, it is advised to include linux/types.h.
  
  This fixes 5 headers_check warnings.
  
 How many times does this need to be NAKed?

Until someone gets a clue and puts comments in there explaining this?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Dave Airlie
On Thu, Dec 2, 2010 at 5:38 AM, Andrew Morton a...@linux-foundation.org wrote:
 On Wed, 1 Dec 2010 17:54:18 +0100
 Julien Cristau jcris...@debian.org wrote:

 On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:

  For headers that get exported to userland and make use of u32 style
  type names, it is advised to include linux/types.h.
 
  This fixes 5 headers_check warnings.
 
 How many times does this need to be NAKed?

 Until someone gets a clue and puts comments in there explaining this?

how about someone fixing the dumb scripts to understand that C header
includes aren't single level.

Like 10 people have posted this patch and not one has come back with a
fix for the app after I pointed it out, like really if people think
they can write C good enough to send kernel patches,
maybe they could put some more effort in and actually fix a real problem.

We should start hashing signed-off-by's so people can't get any glory from them.

Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Intel-gfx] [PATCH 1/4] drm: fix headers to include linux/types.h

2010-12-01 Thread Randy Dunlap
On Thu, 2 Dec 2010 05:55:59 +1000 Dave Airlie wrote:

 On Thu, Dec 2, 2010 at 5:38 AM, Andrew Morton a...@linux-foundation.org 
 wrote:
  On Wed, 1 Dec 2010 17:54:18 +0100
  Julien Cristau jcris...@debian.org wrote:
 
  On Wed, Dec  1, 2010 at 17:10:42 +0200, Alexander Shishkin wrote:
 
   For headers that get exported to userland and make use of u32 style
   type names, it is advised to include linux/types.h.
  
   This fixes 5 headers_check warnings.
  
  How many times does this need to be NAKed?
 
  Until someone gets a clue and puts comments in there explaining this?
 
 how about someone fixing the dumb scripts to understand that C header
 includes aren't single level.

Still, drm is an exception here, so it needs to be documented as such.


 Like 10 people have posted this patch and not one has come back with a
 fix for the app after I pointed it out, like really if people think
 they can write C good enough to send kernel patches,
 maybe they could put some more effort in and actually fix a real problem.
 
 We should start hashing signed-off-by's so people can't get any glory from 
 them.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel