Re: gnumach 1.3 + gcc 3.3

[Marco Gerards]

"Alfred M. Szmidt" <ams@kemisten.nu> writes:

> Comments inlined.  I might have missed a couple of things since there
> is a lot of small errors in this patch.

Great!

>    2004-01-20  Marco Gerards  <metgerards@student.han.nl>
>
>          * NEWS: Mention the changes since the GNUMach 1.3 release.
>                                                  ^^^^^^^
> s/GNUMach/GNU Mach/

I will just remove the NEWS update, it makes it harder to get the
patch in I guess and it just is not that important for now.  Or does
someone want me to keep it in?

>          * aclocal.m4: Don't call AC_PROG_CC_WORKS_LOCAL and AC_PROG_CC_GNU
>          anymore.  Remove related tests.
>
> What is this change?  You only removed AC_PROG_CC_LOCAL and
> AC_PROG_CC_WORKS_LOCAL from aclocal.m4, you didn't do any of these
> changes.  AC_PROG_CC_GNU and AC_PROG_CC_WORKS_LOCAL are called from
> AC_PROG_CC_LOCAL; maybe you got confused by the indentation.

You are right, the indentation confused me.

> Please double check that your ChangeLog is indeed correct...
>
>          (AC_PROG_CC_LOCAL): Macro removed.
>          (AC_PROG_CC_WORKS_LOCAL): Likewise.
>          * configure.in: Update AC_INIT to contain the version, bugaddress
>                                                                   ^^^^^^^^^^
>          and programname.  Changed the version to 1.4.  Use
>                ^^^^^^^^^^^
>
> This is mostly personal taste, "program name" is easier on the eyes
> then "programname", the same goes for bugaddress.

Oh, I can change this.

>          * i386/i386at/kd.c (kdinit): Initialize the input buffer.
>
> I would suggest that you move this into a seperate "ChangeLog entry",
> since it is unreleated to the autoconf changes.

This was applied to CVS a while ago, this part of the patch will be
removed.

>    -AC_CHECK_TOOL(LD, ld)
>    +AC_CHECK_TOOL(LD, ld)AC_SUBST(LD_FLAGS)
>
> Split it into two lines, no need to obfuscate code like this.

This was copied one on one from HEAD, I wondered if there was a reason
for this.  I guess not. :)

> By the way, shouldn't this be s/LD_FLAGS/LDFLAGS/?

Ehm, I'll have a look at that.

I will make the changes and send in a new patch soon.

Thanks,
Marco