code review 13261050: cmd/api: include constant values

cmd/api: include constant values

Update Issue 5935

[bradfitz, 2013-09-05 19:45:48]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2013-09-05 19:55:45]

LGTM

Do you need to update api.txt too?

[bradfitz, 2013-09-05 19:58:46]

Will do it in subsequent CLs.



On Thu, Sep 5, 2013 at 12:55 PM, Russ Cox <rsc@golang.org> wrote:

> LGTM
>
> Do you need to update api.txt too?
>
>

[bradfitz, 2013-09-05 20:59:20]

I ran this on a Go 1.1 tree (Go 1.0 was too painful to make go/types work
on, at least how I was doing it) and the only differences were:

+pkg os (linux-arm), const O_SYNC = 1052672
-pkg os (linux-arm), const O_SYNC = 4096

+pkg os (linux-arm-cgo), const O_SYNC = 1052672
-pkg os (linux-arm-cgo), const O_SYNC = 4096

-pkg syscall (darwin-386), const ImplementsGetwd = false
+pkg syscall (darwin-386), const ImplementsGetwd = true

The final item is explainable, and I think we're cool with that.  I can add
that to except.txt.

I'm not sure why O_SYNC changed, though.

Do we care about running this diff against 1.0, considering that 1.1 is
already out?  If we broke const promises from 1.0 to 1.1, the damage is
already done.



On Thu, Sep 5, 2013 at 12:58 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote:

> Will do it in subsequent CLs.
>
>
>
> On Thu, Sep 5, 2013 at 12:55 PM, Russ Cox <rsc@golang.org> wrote:
>
>> LGTM
>>
>> Do you need to update api.txt too?
>>
>>
>

[rsc, 2013-09-05 21:03:30]

I think you're right about 1.1 being a reasonable base.
I'm not thrilled about ImplementsGetwd changing but at least it's not a
number, so it cannot appear in type definitions like [O_SYNC]int.

Let's figure out O_SYNC on arm. Maybe the 0x100000 bit means something else?

Russ

[iant, 2013-09-05 23:18:57]

On Thu, Sep 5, 2013 at 2:03 PM, Russ Cox <rsc@golang.org> wrote:
> I think you're right about 1.1 being a reasonable base.
> I'm not thrilled about ImplementsGetwd changing but at least it's not a
> number, so it cannot appear in type definitions like [O_SYNC]int.
>
> Let's figure out O_SYNC on arm. Maybe the 0x100000 bit means something else?

On GNU/Linux systems O_SYNC used to be 010000.  But then Linux added
the ability to separately sync data operations and read operations.
So they renamed O_SYNC to O_DSYNC, and changed O_SYNC to 04010000 to
do both kinds of syncing.

Here is the bug for the Linux Standard Base, which naturally faced the
same issue: https://lsbbugs.linuxfoundation.org/show_bug.cgi?id=3542 .

Some more background at
http://www.gossamer-threads.com/lists/linux/kernel/1122937 .

Ian

[iant, 2013-09-05 23:20:07]

On Thu, Sep 5, 2013 at 4:18 PM, Ian Lance Taylor <iant@golang.org> wrote:
> On Thu, Sep 5, 2013 at 2:03 PM, Russ Cox <rsc@golang.org> wrote:
>> I think you're right about 1.1 being a reasonable base.
>> I'm not thrilled about ImplementsGetwd changing but at least it's not a
>> number, so it cannot appear in type definitions like [O_SYNC]int.
>>
>> Let's figure out O_SYNC on arm. Maybe the 0x100000 bit means something else?
>
> On GNU/Linux systems O_SYNC used to be 010000.  But then Linux added
> the ability to separately sync data operations and read operations.
> So they renamed O_SYNC to O_DSYNC, and changed O_SYNC to 04010000 to
> do both kinds of syncing.
>
> Here is the bug for the Linux Standard Base, which naturally faced the
> same issue: https://lsbbugs.linuxfoundation.org/show_bug.cgi?id=3542 .
>
> Some more background at
> http://www.gossamer-threads.com/lists/linux/kernel/1122937 .


And the upshot of this is that there is no right answer for the kind
of compatibility we are trying to provide for Go.  I would vote for
changing the value; I think that is the least worst thing we can do.

Ian

[rsc, 2013-09-06 00:07:55]

Do we need to worry about older systems rejecting the unknown mode bit?

[bradfitz, 2013-09-06 00:16:35]

On Thu, Sep 5, 2013 at 5:07 PM, Russ Cox <rsc@golang.org> wrote:

> Do we need to worry about older systems rejecting the unknown mode bit?
>

Looking at the kernel's fs/open.c do_sys_open and build_open_flags and
do_filp_open, I don't see any checks for unknown flag bits.

Also: you'd imagine the libc people would've thought of this too when they
changed the kernel.  The Linux kernel people are pretty strict about not
breaking userspace.

[dave_cheney.net, 2013-09-06 01:06:12]

Arm also went through a major upheaval from OABI to EABI which was an
opportunity to break a bunch of things.

On the oldest arm system I have, armv5 I find this

/*
 * Before Linux 2.6.33 only O_DSYNC semantics were implemented, but using
 * the O_SYNC flag.  We continue to use the existing numerical value
 * for O_DSYNC semantics now, but using the correct symbolic name for it.
 * This new value is used to request true Posix O_SYNC semantics.  It is
 * defined in this strange way to make sure applications compiled against
 * new headers get at least O_DSYNC semantics on older kernels.
 *
 * This has the nice side-effect that we can simply test for O_DSYNC
 * wherever we do not care if O_DSYNC or O_SYNC is used.
 *
 * Note: __O_SYNC must never be used directly.
 */
#ifndef O_SYNC
#define __O_SYNC        04000000
#define O_SYNC          (__O_SYNC|O_DSYNC)
#endif

We require at least 2.6.35 (maybe .37) for linux/arm so I think we are ok here.

On Fri, Sep 6, 2013 at 10:16 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> On Thu, Sep 5, 2013 at 5:07 PM, Russ Cox <rsc@golang.org> wrote:
>>
>> Do we need to worry about older systems rejecting the unknown mode bit?
>
>
> Looking at the kernel's fs/open.c do_sys_open and build_open_flags and
> do_filp_open, I don't see any checks for unknown flag bits.
>
> Also: you'd imagine the libc people would've thought of this too when they
> changed the kernel.  The Linux kernel people are pretty strict about not
> breaking userspace.
>
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

[bradfitz, 2013-09-06 19:01:00]

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

cmd/api: include constant values

Update Issue 5935

R=golang-dev, rsc, iant, dave
CC=golang-dev
https://codereview.appspot.com/13261050