multiboot2.h includes stdint.h - not good

multiboot2.h includes stdint.h - not good

[Pavel Roskin]

Hello!

The current GRUB reports this on stderr during compilation:

In file included from loader/i386/multiboot_helper.S:21:
./include/multiboot2.h:37:20: error: stdint.h: No such file or directory
In file included from loader/i386/multiboot_helper.S:21:
./include/multiboot2.h:37:20: error: stdint.h: No such file or directory
In file included from loader/i386/multiboot_helper.S:21:
./include/multiboot2.h:37:20: error: stdint.h: No such file or directory

There are two problems here.  One is that stdint.h cannot be found
(apparently, the default includes are not used when compiling for the
target).  Another problem is that the build succeeds despite such
errors.

I'm compiling in x86_64 environment (Fedora 10).  There are no arguments
to configure, so it's compiling for i386-pc.  glibc-devel-2.9-3.i386 is
installed.  /usr/include/stdint.h is present (provided by
glibc-headers-2.9-3.x86_64).

By the way, I already noticed that the inclusion of stdint.h from
multiboot2.h is one of the problems preventing cross-compilation of GRUB
without the libc for the target.  Perhaps GRUB should provide its
replacement for stdint.h when compiling for the target.  Alternatively,
multiboot2.h should use GRUB types.

-- 
Regards,
Pavel Roskin

Re: multiboot2.h includes stdint.h - not good

[Vesa Jääskeläinen]

Pavel Roskin wrote:
> By the way, I already noticed that the inclusion of stdint.h from
> multiboot2.h is one of the problems preventing cross-compilation of GRUB
> without the libc for the target.  Perhaps GRUB should provide its
> replacement for stdint.h when compiling for the target.  Alternatively,
> multiboot2.h should use GRUB types.

In grub ecosystem grub_* types should be used to specify arch dependent
types. Where as I favor stdint.h types like uint8_t for many things, the
idea for grub's own replacement is to easily detect wrong types used on
compilation process.

So... stdint.h out... grub types in..

Re: multiboot2.h includes stdint.h - not good

[Pavel Roskin]

On Mon, 2009-03-23 at 20:43 +0200, Vesa Jääskeläinen wrote:
> Pavel Roskin wrote:
> > By the way, I already noticed that the inclusion of stdint.h from
> > multiboot2.h is one of the problems preventing cross-compilation of GRUB
> > without the libc for the target.  Perhaps GRUB should provide its
> > replacement for stdint.h when compiling for the target.  Alternatively,
> > multiboot2.h should use GRUB types.
> 
> In grub ecosystem grub_* types should be used to specify arch dependent
> types. Where as I favor stdint.h types like uint8_t for many things, the
> idea for grub's own replacement is to easily detect wrong types used on
> compilation process.
> 
> So... stdint.h out... grub types in..

OK, let's fix the error messages for now.

It turns out *.lst files for *.S files are generated without ASM_FILE
defined.  Real compilation defines ASM_FILE.  Several *.S files define
ASM_FILE internally, but loader/i386/multiboot_helper.S is not one of
them.

It's hard to capture errors in the *.lst generation without splitting a
pipe into separate commands and introducing a temporary file, so it's
probably not worth the trouble.  Besides, any errors in the list
generation will be caught at the compilation stage if the compiler flags
are the same.  But they are not the same, and that's a problem.

ASM_FILE should be consistently defined for the same files in the
compilation rules and in the list generating rules in genmk.rb.

ChangeLog:

	* genmk.rb: Define ASM_FILE for *.S files when generating
	any lists.

Index: genmk.rb
===================================================================
--- genmk.rb	(revision 2043)
+++ genmk.rb	(working copy)
@@ -159,17 +159,17 @@
 
 #{command}: #{src} $(#{src}_DEPENDENCIES) gencmdlist.sh
 	set -e; \
-	  $(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) $(TARGET_#{flag}) $(#{prefix}_#{flag}) -E $< \
+	  $(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) #{extra_flags} $(TARGET_#{flag}) $(#{prefix}_#{flag}) -E $< \
 	  | sh $(srcdir)/gencmdlist.sh #{symbolic_name} > $@ || (rm -f $@; exit 1)
 
 #{fs}: #{src} $(#{src}_DEPENDENCIES) genfslist.sh
 	set -e; \
-	  $(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) $(TARGET_#{flag}) $(#{prefix}_#{flag}) -E $< \
+	  $(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) #{extra_flags} $(TARGET_#{flag}) $(#{prefix}_#{flag}) -E $< \
 	  | sh $(srcdir)/genfslist.sh #{symbolic_name} > $@ || (rm -f $@; exit 1)
 
 #{partmap}: #{src} $(#{src}_DEPENDENCIES) genpartmaplist.sh
 	set -e; \
-	  $(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) $(TARGET_#{flag}) $(#{prefix}_#{flag}) -E $< \
+	  $(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) #{extra_flags} $(TARGET_#{flag}) $(#{prefix}_#{flag}) -E $< \
 	  | sh $(srcdir)/genpartmaplist.sh #{symbolic_name} > $@ || (rm -f $@; exit 1)
 
 


-- 
Regards,
Pavel Roskin