Re: RISC OS port

[Timothy Baldwin]

On Friday 03 Dec 2004 13:07, Marco Gerards wrote:
> Timothy Baldwin <address@hidden> writes:
> 
> > On Saturday 27 Nov 2004 01:36, I wrote:
> >
> >> 1. Portability fixes and configure.ac changes.
> >
> > Here they are. However cross-compiling is broken in that
> > the utilities are only built for the build system, without
> > this patch they are only built for the host system. Either
> > way, the result is not a complete installation.
> 
> Ok.  I really hope someone else can have a look at this because I
> don't know much about autoconf/automake, etc.  However, I am trying to
> comment on your code wherever I can.
> 
> > I propose to solve this problem by running configure twice
> > when cross-compiling, once for the host, once for the build
> > system. This could be done by adding these make rules:
> >
> > build/%: build/Makefile
> >  cd build && $(MAKE) $*
> >  touch $@
> >
> > build/Makefile: $(srcdir)/configure
> >  cd build && $(srcdir)/configure --host=$(build) --target=$(host)
> >
> > Define BUILD to "build/" when cross compiling or to ""
> > when not cross-compiling and prefix use of the utilites
> > in makefiles with $(BUILD).
> >
> > Shall I do that?
> 
> This does not look like a clean solution to me.

> Normally this is not 
> required to do it like this, right?

The gcc package also uses multiple runs of configure.

You're right, genmoddep does not require the services of configure,
but then why was support for checking the build system compiler
added to configure.ac?

I suggest removing that support, and only use BUILD_CC for compiling
genmoddep. Support for building cross versions of the tools could be
retained by supporting configure's --target option. 

But for more complicated cases requiring the use of different toolchains,
(eg a Windows CE port of GRUB, or a Windows (ME) port of grub-setup) I
still recommend running configure multiple times.


> 
> Can you please make a changelog entry for this patch?

Will do.

> > +if test "x$host_cpu" = xarm; then
> > +  AC_PATH_PROG(PERL, perl)
> > +fi
> > +
> 
> Why do you need perl?

A perl program is used to convert the list of machine names 
and numbers from Linux.


> > +    GRUB_ERR_FILE_CLOSE_ERROR,
> > +    GRUB_ERR_FILE_OPEN_ERROR,
> > +    GRUB_ERR_DIR_READ_ERROR
> 
> When are these errors triggered, btw?

When the corresponding RISC OS system call returns an error.

> Can't you use GRUB_ERR_BAD_FS 
> or GRUB_FS_BAD_FILE_TYPE instead?

The error is probably not one of those, and could almost anything.


 
> > +void *EXPORT_FUNC(memcpy) (void *dest, const void *src, grub_size_t n);
> > +void *EXPORT_FUNC(memset) (void *s, int c, grub_size_t n);
> 
> Why do you want to do this?  Can't you just use grub_memcpy and
> grub_memset instead?

They are called implictally by gcc.

> > -#define FUNCTION(x) .globl EXT_C(x) ; .type EXT_C(x), @function ; EXT_C(x):
> > -#define VARIABLE(x) .globl EXT_C(x) ; .type EXT_C(x), @object ; EXT_C(x):
> > +#define FUNCTION(x) .globl EXT_C(x) ; .type EXT_C(x), "function" ; 
> > EXT_C(x):
> > +#define VARIABLE(x) .globl EXT_C(x) ; .type EXT_C(x), "object" ; EXT_C(x):
> 
> Can you please explain this change?

"@" marks the start of a comment in ARM GNU as syntax, producing a syntax
error. I expect double quotes to work for all architures. Tested on x86 and 
PowerPC.

> >  }
> > +
> >  void *memmove (void *dest, const void *src, grub_size_t n)
> >    __attribute__ ((alias ("grub_memmove")));
> > +
> 
> Please watch out with adding white lines.

IMO, there should be white space there to make it more readable.

> Does GRUB 2 still work on the PC after this patch is committed?

Yes
> 
> Thanks,
> Marco
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 

-- 
Member AFFS, WYLUG, SWP (UK), ANL, RESPECT, Leeds SA, Leeds Anti-war coalition
No to software patents!    Victory to the iraqi resistance!

pgpSvf6ojNYAf.pgp
Description: PGP signature