BUG: via_drmclient.h is referenced but does not exist

BUG: via_drmclient.h is referenced but does not exist

[Sam Ravnborg]

Hi David et al.

After improving the headers_check.pl script to do a
check for files included with:

	#include "foo.h"

it reported the following error:

/home/sam/kernel/knext.git/usr/include/drm/via_drm.h:34: included file '/home/sam/kernel/knext.git/usr/include/drm/via_drmclient.h' is not exported

And indeed the file via_drmclient.h does not exist in the kernel source.
We do not see any issue in the kernel as the include is guarded by:

#ifndef __KERNEL__
#include "via_drmclient.h"
#endif

But referring to a non-existing file is not the right thing to to.

We need this check as we have several places where we do:

	#include "foo.h"

in our exported headers.

So we need to fix drm somehow.

The headers_check.pl patch is included for reference.

	Sam

diff --git a/scripts/headers_check.pl b/scripts/headers_check.pl
index 488a3b1..b97341e 100644
--- a/scripts/headers_check.pl
+++ b/scripts/headers_check.pl
@@ -32,7 +32,7 @@ foreach my $file (@files) {
 	$lineno = 0;
 	while ($line = <FH>) {
 		$lineno++;
-		check_include();
+		check_include($filename);
 	}
 	close FH;
 }
@@ -40,7 +40,8 @@ exit $ret;
 
 sub check_include
 {
-	if ($line =~ m/^\s*#\s*include\s+<((asm|linux).*)>/) {
+	my ($filename) = @_;
+	if ($line =~ m/^\s*#\s*include\s+<((asm|linux)[_a-zA-Z0-9\/.]+)>/) {
 		my $inc = $1;
 		my $found;
 		$found = stat($dir . "/" . $inc);
@@ -52,5 +53,18 @@ sub check_include
 			printf STDERR "$filename:$lineno: included file '$inc' is not exported\n";
 			$ret = 1;
 		}
+	} elsif ($line =~ m/^\s*#\s*include\s+"([_a-zA-Z0-9.]+)"/) {
+		my $inc = $1;
+		my $filedir = $filename;
+		my $found;
+
+		# drop filename part so we have only the dir part
+		$filedir =~ s/\/[_a-zA-Z0-9.]+$//;
+
+		$found = stat($filedir . "/" . $inc);
+		if (!$found) {
+			printf STDERR "$filename:$lineno: included file '$filedir/$inc' is not exported\n";
+			$ret = 1;
+		}
 	}
 }

Re: BUG: via_drmclient.h is referenced but does not exist

[Thomas Hellström]

Hi!

The intention is for via_drm.h to be self-containing when included both 
for a kernel build and for a user-space build. In this particular case, 
via_drmclient.h lives in the user-space clients and includes stdint.h to 
get access to uint32_t and friends.

Of course, the user-space clients could
#include "uint32.h"
#include "via_drm.h"

but shouldn't really the tools be mimicing what the compiler does in 
this case?

/Thomas


Sam Ravnborg wrote:
> Hi David et al.
>
> After improving the headers_check.pl script to do a
> check for files included with:
>
> 	#include "foo.h"
>
> it reported the following error:
>
> /home/sam/kernel/knext.git/usr/include/drm/via_drm.h:34: included file '/home/sam/kernel/knext.git/usr/include/drm/via_drmclient.h' is not exported
>
> And indeed the file via_drmclient.h does not exist in the kernel source.
> We do not see any issue in the kernel as the include is guarded by:
>
> #ifndef __KERNEL__
> #include "via_drmclient.h"
> #endif
>
> But referring to a non-existing file is not the right thing to to.
>
> We need this check as we have several places where we do:
>
> 	#include "foo.h"
>
> in our exported headers.
>
> So we need to fix drm somehow.
>
> The headers_check.pl patch is included for reference.
>
>   

Re: BUG: via_drmclient.h is referenced but does not exist

[Sam Ravnborg]

On Sun, Dec 14, 2008 at 04:57:34PM +0100, Thomas Hellström wrote:
> Hi!
> 
> The intention is for via_drm.h to be self-containing when included both 
> for a kernel build and for a user-space build. In this particular case, 
> via_drmclient.h lives in the user-space clients and includes stdint.h to 
> get access to uint32_t and friends.
> 
> Of course, the user-space clients could
> #include "uint32.h"
> #include "via_drm.h"
> 
> but shouldn't really the tools be mimicing what the compiler does in 
> this case?

The kernel headers and thus the kernel ABI is separate and ideally
they should not depend on any other header files to provide anything.

This is why __u32, __u64 etc are preferred in the kernel ABI
and not uint32_t as used by the drm headers.

We do not adhere to this as a strict rule (yet).
But if you do:

grep -l uint32_t usr/include/linux

then you will only see 7 hits. Out of 368 files.
So we are not bad in this respect.

For drm the fix seems simple - just replace all of uint32_t with __u32.
likewise for the other 32 bit and the 64 bit variants.

For the specific case where drm includes a non-existing file I suggest
that we get this fixed in some way soon.

	Sam

Re: BUG: via_drmclient.h is referenced but does not exist

[Thomas Hellström]

Hi!

Sam Ravnborg wrote:
> On Sun, Dec 14, 2008 at 04:57:34PM +0100, Thomas Hellström wrote:
>   
>> Hi!
>>
>> The intention is for via_drm.h to be self-containing when included both 
>> for a kernel build and for a user-space build. In this particular case, 
>> via_drmclient.h lives in the user-space clients and includes stdint.h to 
>> get access to uint32_t and friends.
>>
>> Of course, the user-space clients could
>> #include "uint32.h"
>> #include "via_drm.h"
>>
>> but shouldn't really the tools be mimicing what the compiler does in 
>> this case?
>>     
>
> The kernel headers and thus the kernel ABI is separate and ideally
> they should not depend on any other header files to provide anything.
>
> This is why __u32, __u64 etc are preferred in the kernel ABI
> and not uint32_t as used by the drm headers.
>
> We do not adhere to this as a strict rule (yet).
> But if you do:
>
> grep -l uint32_t usr/include/linux
>
> then you will only see 7 hits. Out of 368 files.
> So we are not bad in this respect.
>
> For drm the fix seems simple - just replace all of uint32_t with __u32.
> likewise for the other 32 bit and the 64 bit variants.
>   
I'm not sure that's possible, as these header files are shared with *BSD 
(and possibly Solaris).
> For the specific case where drm includes a non-existing file I suggest
> that we get this fixed in some way soon.
>   
It's a bit non-trivial, since all via drm user-space clients need to be 
fixed, but definitiely doable.
/Thomas

Re: BUG: via_drmclient.h is referenced but does not exist

[Sam Ravnborg]

On Mon, Dec 15, 2008 at 10:26:27PM +0100, Thomas Hellström wrote:
> Hi!
> 
> Sam Ravnborg wrote:
> >On Sun, Dec 14, 2008 at 04:57:34PM +0100, Thomas Hellström wrote:
> >  
> >>Hi!
> >>
> >>The intention is for via_drm.h to be self-containing when included both 
> >>for a kernel build and for a user-space build. In this particular case, 
> >>via_drmclient.h lives in the user-space clients and includes stdint.h to 
> >>get access to uint32_t and friends.
> >>
> >>Of course, the user-space clients could
> >>#include "uint32.h"
> >>#include "via_drm.h"
> >>
> >>but shouldn't really the tools be mimicing what the compiler does in 
> >>this case?
> >>    
> >
> >The kernel headers and thus the kernel ABI is separate and ideally
> >they should not depend on any other header files to provide anything.
> >
> >This is why __u32, __u64 etc are preferred in the kernel ABI
> >and not uint32_t as used by the drm headers.
> >
> >We do not adhere to this as a strict rule (yet).
> >But if you do:
> >
> >grep -l uint32_t usr/include/linux
> >
> >then you will only see 7 hits. Out of 368 files.
> >So we are not bad in this respect.
> >
> >For drm the fix seems simple - just replace all of uint32_t with __u32.
> >likewise for the other 32 bit and the 64 bit variants.
> >  
> I'm not sure that's possible, as these header files are shared with *BSD 
> (and possibly Solaris).
I dunno about *BSD and solaris.

But other linux ABI header files do:

#include <linux/types.h>

To get access to these basic types.
And we could do the same for drm if the other systems does not prevent it.

	Sam