code review 49260043: libmach: use Go's ureg headers on Plan 9

libmach: use Go's ureg headers on Plan 9

    The CL 49090043 renamed Ureg structures to Ureg386,
    UregArm and UregAmd64. This broke build on Plan 9,
    since ureg_x86.h includes /386/include/ureg.h, which
    declares a structure named Ureg instead of Ureg386.

[0intro, 2014-01-08 21:14:32]

Hello golang-codereviews@googlegroups.com (cc: rsc),

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2014-01-08 21:20:05]

I thought the plan9 version of Go was moving towards just using the Go
libraries and not mixing & and matching parts of Go vs plan9 system
libraries/headers?  Thought I saw a reference to that once.

I'm not really following too closely.



On Wed, Jan 8, 2014 at 1:14 PM, <0intro@gmail.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com (cc: rsc),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
>     libmach: fix build on Plan 9 after Ureg renaming
>
>     The CL 49090043 renamed Ureg structures to Ureg386,
> UregArm and UregAmd64. This broke build on Plan 9,
> since ureg_x86.h includes /386/include/ureg.h, which
> includes a structure named Ureg instead of Ureg386.
>
> Please review this at https://codereview.appspot.com/49260043/
>
> Affected files (+6, -0 lines):
>   M include/plan9/ureg_amd64.h
>   M include/plan9/ureg_arm.h
>   M include/plan9/ureg_x86.h
>
>
> Index: include/plan9/ureg_amd64.h
> ===================================================================
> --- a/include/plan9/ureg_amd64.h
> +++ b/include/plan9/ureg_amd64.h
> @@ -2,4 +2,6 @@
>  // Use of this source code is governed by a BSD-style
>  // license that can be found in the LICENSE file.
>
> +#define Ureg UregAmd64
>  #include "/amd64/include/ureg.h"
> +#undef Ureg
> Index: include/plan9/ureg_arm.h
> ===================================================================
> --- a/include/plan9/ureg_arm.h
> +++ b/include/plan9/ureg_arm.h
> @@ -2,4 +2,6 @@
>  // Use of this source code is governed by a BSD-style
>  // license that can be found in the LICENSE file.
>
> +#define Ureg UregArm
>  #include "/arm/include/ureg.h"
> +#undef Ureg
> Index: include/plan9/ureg_x86.h
> ===================================================================
> --- a/include/plan9/ureg_x86.h
> +++ b/include/plan9/ureg_x86.h
> @@ -2,4 +2,6 @@
>  // Use of this source code is governed by a BSD-style
>  // license that can be found in the LICENSE file.
>
> +#define Ureg Ureg386
>  #include "/386/include/ureg.h"
> +#undef Ureg
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[0intro, 2014-01-08 21:30:24]

> I thought the plan9 version of Go was moving towards just using the Go
> libraries and not mixing & and matching parts of Go vs plan9 system
> libraries/headers?  Thought I saw a reference to that once.

Yes, I think you're right and we should really use Go ureg headers instead.
I've updated the CL.

[bradfitz, 2014-01-08 21:31:18]

LGTM

[0intro, 2014-01-08 22:34:18]

*** Submitted as https://code.google.com/p/go/source/detail?r=db95cbe91420 ***

    libmach: use Go's ureg headers on Plan 9

    The CL 49090043 renamed Ureg structures to Ureg386,
    UregArm and UregAmd64. This broke build on Plan 9,
    since ureg_x86.h includes /386/include/ureg.h, which
    declares a structure named Ureg instead of Ureg386.

R=golang-codereviews, bradfitz
CC=golang-codereviews, rsc
https://codereview.appspot.com/49260043

[rsc, 2014-01-09 01:08:30]

LGTM

Can't you just delete these files?

[0intro, 2014-01-09 06:40:18]

> Can't you just delete these files?

In the present state, we can't, since we don't include
the "include" directory at all.

We could include both the "include" and "include/plan9"
directories, but there are still cases were we currently
use system headers instead of Go ones, like bootexec.h.

Using Go's bootexec.h would fail because of the empty
structures, but it could be worked around with a couple
of #ifdef PLAN9 in bootexec.h.

Should I propose a CL for that?

-- 
David du Colombier

[0intro, 2014-01-09 08:22:27]

I've created CL 49680043, which should remove the need of #include
"../xxx.h" headers. I'm not sure it is worth however.

-- 
David du Colombier

[rsc, 2014-01-09 14:34:48]

Don't worry about it then. Libmach will be gone soon.