2012-02-23 19:02:24

by Bobby Powers

[permalink] [raw]
Subject: [PATCH] linux headers: include linux/types.h where appropriate

This silences the related header check warnings.

Signed-off-by: Bobby Powers <[email protected]>
---
include/drm/drm_mode.h | 2 ++
include/drm/i915_drm.h | 2 ++
include/drm/mga_drm.h | 2 ++
include/drm/radeon_drm.h | 2 ++
include/drm/via_drm.h | 2 ++
include/linux/mmc/ioctl.h | 3 +++
include/scsi/scsi_netlink.h | 2 +-
include/sound/compress_params.h | 2 ++
8 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 2a2acda..4a0aae3 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 924f6a4..74b0d6c 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -29,6 +29,8 @@

#include "drm.h"

+#include <linux/types.h>
+
/* Please note that modifications to all structs defined here are
* subject to backwards-compatibility constraints.
*/
diff --git a/include/drm/mga_drm.h b/include/drm/mga_drm.h
index fca8170..6d4f427 100644
--- a/include/drm/mga_drm.h
+++ b/include/drm/mga_drm.h
@@ -37,6 +37,8 @@

#include "drm.h"

+#include <linux/types.h>
+
/* WARNING: If you change any of these defines, make sure to change the
* defines in the Xserver file (mga_sarea.h)
*/
diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
index b55da40..fdfedd2 100644
--- a/include/drm/radeon_drm.h
+++ b/include/drm/radeon_drm.h
@@ -35,6 +35,8 @@

#include "drm.h"

+#include <linux/types.h>
+
/* WARNING: If you change any of these defines, make sure to change the
* defines in the X server file (radeon_sarea.h)
*/
diff --git a/include/drm/via_drm.h b/include/drm/via_drm.h
index 79b3b6e..c6d0992 100644
--- a/include/drm/via_drm.h
+++ b/include/drm/via_drm.h
@@ -26,6 +26,8 @@

#include "drm.h"

+#include <linux/types.h>
+
/* WARNING: These defines must be the same as what the Xserver uses.
* if you change them, you must change the defines in the Xserver.
*/
diff --git a/include/linux/mmc/ioctl.h b/include/linux/mmc/ioctl.h
index 8fa5bc5..1f5e689 100644
--- a/include/linux/mmc/ioctl.h
+++ b/include/linux/mmc/ioctl.h
@@ -1,5 +1,8 @@
#ifndef LINUX_MMC_IOCTL_H
#define LINUX_MMC_IOCTL_H
+
+#include <linux/types.h>
+
struct mmc_ioc_cmd {
/* Implies direction of data. true = write, false = read */
int write_flag;
diff --git a/include/scsi/scsi_netlink.h b/include/scsi/scsi_netlink.h
index 58ce8fe..5cb20cc 100644
--- a/include/scsi/scsi_netlink.h
+++ b/include/scsi/scsi_netlink.h
@@ -23,7 +23,7 @@
#define SCSI_NETLINK_H

#include <linux/netlink.h>
-
+#include <linux/types.h>

/*
* This file intended to be included by both kernel and user space
diff --git a/include/sound/compress_params.h b/include/sound/compress_params.h
index d97d69f..da4a456 100644
--- a/include/sound/compress_params.h
+++ b/include/sound/compress_params.h
@@ -51,6 +51,8 @@
#ifndef __SND_COMPRESS_PARAMS_H
#define __SND_COMPRESS_PARAMS_H

+#include <linux/types.h>
+
/* AUDIO CODECS SUPPORTED */
#define MAX_NUM_CODECS 32
#define MAX_NUM_CODEC_DESCRIPTORS 32
--
1.7.7.6


2012-02-23 19:28:13

by Dave Airlie

[permalink] [raw]
Subject: Re: [PATCH] linux headers: include linux/types.h where appropriate


> This silences the related header check warnings.

please fix the bug in the correct place, the drm.h header includes
linux/types.h and this is one on purpose.

maybe expend some effort fixing the stupid tool to recurse like a normal
preprocessor would.

Dave.

2012-02-23 19:39:45

by Bobby Powers

[permalink] [raw]
Subject: Re: [PATCH] linux headers: include linux/types.h where appropriate

On Thu, Feb 23, 2012 at 2:28 PM, Dave Airlie <[email protected]> wrote:
>
>> This silences the related header check warnings.
>
> please fix the bug in the correct place, the drm.h header includes
> linux/types.h and this is one on purpose.
>
> maybe expend some effort fixing the stupid tool to recurse like a normal
> preprocessor would.

Hah fair enough, I'll look into that later today then.

2012-02-23 19:50:30

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] linux headers: include linux/types.h where appropriate

On Thu, Feb 23, 2012 at 07:28:10PM +0000, Dave Airlie wrote:
>
> > This silences the related header check warnings.
>
> please fix the bug in the correct place, the drm.h header includes
> linux/types.h and this is one on purpose.
>
> maybe expend some effort fixing the stupid tool to recurse like a normal
> preprocessor would.

Before we fix the tool maybe we should agree on what the tool should check for.
One obvious thing to check is that we do not start to use user-land
types in our exported headers like uint32_t etc.
Another could be that no header file should depend oon the user to include other
header files.
Etc.

The checks we have implmented today are very conservative - but as you resist
to add a few includes in the drm/drm*.h files prevents us from hitting
zero warnings even with these conservative checks.

If we started to be more strict we would see much more warnings/errors.
But all this fuzz about a handfull of includes in the drm header files
sure scare me away from ever starting this.
I can only image the uproar some other changes may cause then.

Sam

2012-02-23 21:27:41

by Bobby Powers

[permalink] [raw]
Subject: Re: [PATCH] linux headers: include linux/types.h where appropriate

On Thu, Feb 23, 2012 at 2:50 PM, Sam Ravnborg <[email protected]> wrote:
> On Thu, Feb 23, 2012 at 07:28:10PM +0000, Dave Airlie wrote:
>>
>> > This silences the related header check warnings.
>>
>> please fix the bug in the correct place, the drm.h header includes
>> linux/types.h and this is one on purpose.
>>
>> maybe expend some effort fixing the stupid tool to recurse like a normal
>> preprocessor would.
>
> Before we fix the tool maybe we should agree on what the tool should check for.
> One obvious thing to check is that we do not start to use user-land
> types in our exported headers like uint32_t etc.
> Another could be that no header file should depend oon the user to include other
> header files.
> Etc.

I just sent out a patch that recursively checks for including types.h,
as it was quite simple. Additional checks seem like they may be a
good idea, but not something that has to be decided right here.

> The checks we have implmented today are very conservative - but as you resist
> to add a few includes in the drm/drm*.h files prevents us from hitting
> zero warnings even with these conservative checks.
>
> If we started to be more strict we would see much more warnings/errors.
> But all this fuzz about a handfull of includes in the drm header files
> sure scare me away from ever starting this.
> I can only image the uproar some other changes may cause then.

Interestingly, with headers_check searching for types.h recursively
(clearing up some noise), include/drm/drm_mode.h stands out as still
needing to include <linux/types.h>, as it doesn't import drm.h.

>
> ? ? ? ?Sam