[PATCH] include <linux/fs.h> into linux/ext2_fs.h

[PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Kirill A. Shutemov]

linux/fs.h defines FS_IOC* needed by linux/ext2_fs.h

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: <linux-ext4@vger.kernel.org>
---
 include/linux/ext2_fs.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
index 2efe7b8..f23ba68 100644
--- a/include/linux/ext2_fs.h
+++ b/include/linux/ext2_fs.h
@@ -18,6 +18,7 @@
 
 #include <linux/types.h>
 #include <linux/magic.h>
+#include <linux/fs.h>
 
 /*
  * The second extended filesystem constants/structures
-- 
1.5.6.5.GIT

[PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[Kirill A. Shutemov]

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Christine Caulfield <caulfie@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/dlm_plock.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/dlm_plock.h b/include/linux/dlm_plock.h
index 18d5fdb..2dd2124 100644
--- a/include/linux/dlm_plock.h
+++ b/include/linux/dlm_plock.h
@@ -9,6 +9,8 @@
 #ifndef __DLM_PLOCK_DOT_H__
 #define __DLM_PLOCK_DOT_H__
 
+#include <linux/types.h>
+
 #define DLM_PLOCK_MISC_NAME		"dlm_plock"
 
 #define DLM_PLOCK_VERSION_MAJOR	1
-- 
1.5.6.5.GIT

[PATCH] include <linux/types.h> into linux/if_addrlabel.h for __u* typedef

[Kirill A. Shutemov]

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: David S. Miller <davem@davemloft.net>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/linux/if_addrlabel.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/if_addrlabel.h b/include/linux/if_addrlabel.h
index 9fe79c9..89571f6 100644
--- a/include/linux/if_addrlabel.h
+++ b/include/linux/if_addrlabel.h
@@ -10,6 +10,8 @@
 #ifndef __LINUX_IF_ADDRLABEL_H
 #define __LINUX_IF_ADDRLABEL_H
 
+#include <linux/types.h>
+
 struct ifaddrlblmsg
 {
 	__u8		ifal_family;		/* Address family */
-- 
1.5.6.5.GIT

[PATCH] include <linux/videodev2.h> into linux/ivtv.h

[Kirill A. Shutemov]

linux/videodev2.h defines enum v4l2_buf_type and struct v4l2_rect

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
---
 include/linux/ivtv.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/ivtv.h b/include/linux/ivtv.h
index 17ca64b..f272028 100644
--- a/include/linux/ivtv.h
+++ b/include/linux/ivtv.h
@@ -23,6 +23,7 @@
 
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <linux/videodev2.h>
 
 /* ivtv knows several distinct output modes: MPEG streaming,
    YUV streaming, YUV updates through user DMA and the passthrough
-- 
1.5.6.5.GIT

Re: [PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[Alexey Dobriyan]

On Wed, Sep 10, 2008 at 02:22:04PM +0300, Kirill A. Shutemov wrote:
> --- a/include/linux/dlm_plock.h
> +++ b/include/linux/dlm_plock.h

> +#include <linux/types.h>

I'd say that unless you can demonstrate compilation breakage, these will
only slow down compilation.

Re: [PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On Wed, Sep 10, 2008 at 04:14:50PM +0400, Alexey Dobriyan wrote:
> On Wed, Sep 10, 2008 at 02:22:04PM +0300, Kirill A. Shutemov wrote:
> > --- a/include/linux/dlm_plock.h
> > +++ b/include/linux/dlm_plock.h
> 
> > +#include <linux/types.h>
> 
> I'd say that unless you can demonstrate compilation breakage, these will
> only slow down compilation.

Do you think that minor slow down is worse than dependence on the order of
inclusion?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[Alexey Dobriyan]

On Wed, Sep 10, 2008 at 03:55:04PM +0300, Kirill A. Shutemov wrote:
> On Wed, Sep 10, 2008 at 04:14:50PM +0400, Alexey Dobriyan wrote:
> > On Wed, Sep 10, 2008 at 02:22:04PM +0300, Kirill A. Shutemov wrote:
> > > --- a/include/linux/dlm_plock.h
> > > +++ b/include/linux/dlm_plock.h
> > 
> > > +#include <linux/types.h>
> > 
> > I'd say that unless you can demonstrate compilation breakage, these will
> > only slow down compilation.
> 
> Do you think that minor slow down is worse than dependence on the order of
> inclusion?

Since it compiles fine there is no such dependency.

Just removing externs from function prototypes makes compiles go faster for
_seconds_.

Re: [PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

On Wed, Sep 10, 2008 at 05:11:58PM +0400, Alexey Dobriyan wrote:
> On Wed, Sep 10, 2008 at 03:55:04PM +0300, Kirill A. Shutemov wrote:
> > On Wed, Sep 10, 2008 at 04:14:50PM +0400, Alexey Dobriyan wrote:
> > > On Wed, Sep 10, 2008 at 02:22:04PM +0300, Kirill A. Shutemov wrote:
> > > > --- a/include/linux/dlm_plock.h
> > > > +++ b/include/linux/dlm_plock.h
> > > 
> > > > +#include <linux/types.h>
> > > 
> > > I'd say that unless you can demonstrate compilation breakage, these will
> > > only slow down compilation.
> > 
> > Do you think that minor slow down is worse than dependence on the order of
> > inclusion?
> 
> Since it compiles fine there is no such dependency.

It doesn't compiles:

$ make headers_install
...
$ gcc usr/include/linux/dlm_plock.h
usr/include/linux/dlm_plock.h:25: error: expected specifier-qualifier-list
before ‘__u32’

> Just removing externs from function prototypes makes compiles go faster for
> _seconds_.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=utf-8, Size: 631 bytes --]

From: "Kirill A. Shutemov" <kirill@shutemov.name>
Date: Wed, 10 Sep 2008 16:15:13 +0300

> $ make headers_install
> ...
> $ gcc usr/include/linux/dlm_plock.h
> usr/include/linux/dlm_plock.h:25: error: expected specifier-qualifier-list
> before ‘__u32’

That's not how this file is used by real applications.
Anybody can cook up contrived cases like this.

Please show a real application or kernel compilation which breaks
because of the lack of types.h include here.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

On Wed, Sep 10, 2008 at 01:18:24PM -0700, David Miller wrote:
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
> Date: Wed, 10 Sep 2008 16:15:13 +0300
> 
> > $ make headers_install
> > ...
> > $ gcc usr/include/linux/dlm_plock.h
> > usr/include/linux/dlm_plock.h:25: error: expected specifier-qualifier-list
> > before ‘__u32’
> 
> That's not how this file is used by real applications.
> Anybody can cook up contrived cases like this.
> 
> Please show a real application or kernel compilation which breaks
> because of the lack of types.h include here.

Any application which include this header before linux/types.h (directly or 
indirectly) will get the error. It's dependence on the order of inclusion.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] include <linux/types.h> into linux/dlm_plock.h for __u* typedef

[David Woodhouse]

On Wed, 2008-09-10 at 16:14 +0400, Alexey Dobriyan wrote:
> On Wed, Sep 10, 2008 at 02:22:04PM +0300, Kirill A. Shutemov wrote:
> > --- a/include/linux/dlm_plock.h
> > +++ b/include/linux/dlm_plock.h
> 
> > +#include <linux/types.h>
> 
> I'd say that unless you can demonstrate compilation breakage, these will
> only slow down compilation.

Not really. GCC notices the include guard pattern and won't even bother
to open the file the second and subsequent time it's included within a
compilation. I'd be astonished if you can measure any real slowdown at
all.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Christoph Hellwig]

On Wed, Sep 10, 2008 at 02:22:03PM +0300, Kirill A. Shutemov wrote:
> linux/fs.h defines FS_IOC* needed by linux/ext2_fs.h

And who cares?  

Re: [PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

On Wed, Sep 10, 2008 at 07:27:09AM -0400, Christoph Hellwig wrote:
> On Wed, Sep 10, 2008 at 02:22:03PM +0300, Kirill A. Shutemov wrote:
> > linux/fs.h defines FS_IOC* needed by linux/ext2_fs.h
> 
> And who cares?  

It breaks building pam_mktemp at least.

It's regression introduced by commit 36695673b.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Christoph Hellwig]

On Wed, Sep 10, 2008 at 02:40:12PM +0300, Kirill A. Shutemov wrote:
> On Wed, Sep 10, 2008 at 07:27:09AM -0400, Christoph Hellwig wrote:
> > On Wed, Sep 10, 2008 at 02:22:03PM +0300, Kirill A. Shutemov wrote:
> > > linux/fs.h defines FS_IOC* needed by linux/ext2_fs.h
> > 
> > And who cares?  
> 
> It breaks building pam_mktemp at least.

pam_mktemp should not use <linux/ext2_fs.h>, but rather
<ext2fs/ext2_fs.h>.

Re: [PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On Wed, Sep 10, 2008 at 07:40:42AM -0400, Christoph Hellwig wrote:
> On Wed, Sep 10, 2008 at 02:40:12PM +0300, Kirill A. Shutemov wrote:
> > On Wed, Sep 10, 2008 at 07:27:09AM -0400, Christoph Hellwig wrote:
> > > On Wed, Sep 10, 2008 at 02:22:03PM +0300, Kirill A. Shutemov wrote:
> > > > linux/fs.h defines FS_IOC* needed by linux/ext2_fs.h
> > > 
> > > And who cares?  

Do you think it isn't error?

> > It breaks building pam_mktemp at least.
> 
> pam_mktemp should not use <linux/ext2_fs.h>, but rather
> <ext2fs/ext2_fs.h>.

Why linux/ext2_fs.h does export to userspace?


-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Eric Sandeen]

Kirill A. Shutemov wrote:
> On Wed, Sep 10, 2008 at 07:40:42AM -0400, Christoph Hellwig wrote:
>> On Wed, Sep 10, 2008 at 02:40:12PM +0300, Kirill A. Shutemov wrote:
>>> On Wed, Sep 10, 2008 at 07:27:09AM -0400, Christoph Hellwig wrote:
>>>> On Wed, Sep 10, 2008 at 02:22:03PM +0300, Kirill A. Shutemov wrote:
>>>>> linux/fs.h defines FS_IOC* needed by linux/ext2_fs.h
>>>> And who cares?  
> 
> Do you think it isn't error?
> 
>>> It breaks building pam_mktemp at least.
>> pam_mktemp should not use <linux/ext2_fs.h>, but rather
>> <ext2fs/ext2_fs.h>.

That's right.

> Why linux/ext2_fs.h does export to userspace?
> 

it does?

$ grep header-y include/linux/Kbuild | wc -l
163
$ grep header-y include/linux/Kbuild | grep ext2
$

I think it does not.

-Eric

Re: [PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

On Wed, Sep 10, 2008 at 08:39:00AM -0500, Eric Sandeen wrote:
> Kirill A. Shutemov wrote:
> > On Wed, Sep 10, 2008 at 07:40:42AM -0400, Christoph Hellwig wrote:
> >> On Wed, Sep 10, 2008 at 02:40:12PM +0300, Kirill A. Shutemov wrote:
> >>> On Wed, Sep 10, 2008 at 07:27:09AM -0400, Christoph Hellwig wrote:
> >>>> On Wed, Sep 10, 2008 at 02:22:03PM +0300, Kirill A. Shutemov wrote:
> >>>>> linux/fs.h defines FS_IOC* needed by linux/ext2_fs.h
> >>>> And who cares?  
> > 
> > Do you think it isn't error?
> > 
> >>> It breaks building pam_mktemp at least.
> >> pam_mktemp should not use <linux/ext2_fs.h>, but rather
> >> <ext2fs/ext2_fs.h>.
> 
> That's right.
> 
> > Why linux/ext2_fs.h does export to userspace?
> > 
> 
> it does?
> 
> $ grep header-y include/linux/Kbuild | wc -l
> 163
> $ grep header-y include/linux/Kbuild | grep ext2
> $
> 
> I think it does not.

It does:

$ grep unifdef-y include/linux/Kbuild | grep ext2
unifdef-y += ext2_fs.h

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] include <linux/fs.h> into linux/ext2_fs.h

[Eric Sandeen]

Kirill A. Shutemov wrote:
> On Wed, Sep 10, 2008 at 08:39:00AM -0500, Eric Sandeen wrote:

>> I think it does not.
> 
> It does:
> 
> $ grep unifdef-y include/linux/Kbuild | grep ext2
> unifdef-y += ext2_fs.h

Oops, I stand corrected :)

-Eric