Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added baudrate constants. #530

Merged
merged 9 commits into from
Feb 24, 2017
Merged

Added baudrate constants. #530

merged 9 commits into from
Feb 24, 2017

Conversation

berkowski
Copy link
Contributor

Addresses #528, adding baudrate constants for bsd and notbsd flavors of unix.

Passes libc-test locally on ubuntu 14.04 LTS with the additional (uncommitted) entries into in build.rs:

cfg.skip_const( move |name| {
        match name {
            # ... snip ...
            "PTRACE_O_SUSPEND_SECCOMP" |
            "CLONE_NEWCGROUP" |
            "NETLINK_LIST_MEMBERSHIPS" |
            "NETLINK_LISTEN_ALL_NSID" |
            "NETLINK_CAP_ACK" |
            "PR_CAP_AMBIENT_CLEAR_ALL" |
            "PR_CAP_AMBIENT_LOWER" |
            "PR_CAP_AMBIENT_RAISE" |
            "PR_CAP_AMBIENT_IS_SET" |
            "PR_CAP_AMBIENT" |
            "PR_FP_MODE_FRE" |
            "PR_FP_MODE_FR" |
            "PR_GET_FP_MODE" |
            "PR_SET_FP_MODE" |
            "PR_MPX_DISABLE_MANAGEMENT" |
            "PR_MPX_ENABLE_MANAGEMENT" |
            "PR_GET_THP_DISABLE" |
            "PR_SET_THP_DISABLE" |
            "PR_SET_MM_MAP_SIZE" |
            "PR_GET_MM_MAP_SIZE" |
            "PR_SET_MM_MAP" |
            "NLM_F_DUMP_FILTERED" |
            "EPOLLEXCLUSIVE" => true,

            _ => false,
        }
    });

I'm assuming this is because I'm stuck using linux-libc-dev:3.13.0-24.46 for the moment and those constants are defined in newer versions.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@berkowski
Copy link
Contributor Author

berkowski commented Feb 20, 2017

The ppc-linux failures look like an endian issue to me. I know the ppc is big-endian but I'm not sure why the baud constants seem to remain little-endian for rust in the test. Only immediate difference I see is they're defined as octal literals, not hex or dec. fixed in 43e8557

OpenBSD seems to be reporting the same baud constants as linux? Is this correct? I based my constants off termios-rs and ftp://ftp.usa.openbsd.org/pub/OpenBSD/src/sys/sys/termios.h

@semarie
Copy link
Contributor

semarie commented Feb 20, 2017

about OpenBSD, it is due to rpcsvc/rex.h that define the same names (but with different values). The header is generated from src/lib/librpcsvc/rex.x.

@alexcrichton any way to deal with this situation ? Ideally build.rs should #undef constants from  rpcsvc/rex.h that aren't used and conflicts with termios.h. Is it possible ?

@semarie
Copy link
Contributor

semarie commented Feb 20, 2017

@berkowski could you try to add

cfg.header("termios.h");

inside OpenBSD block (but after cfg.header("rpcsvc/rex.h");) ? it should be enough to redefine values with termios ones.

Attempting to correct for conflicting defines from `rpcsvc/rex.h`
@berkowski
Copy link
Contributor Author

Pulled in the cfg.skip_const changes by accident. Re-pushed with only the header addition. Should still fail though.

Having the B* constants in `unix/notbsd/mod.rs` passed CI tests
except for powerpc.  So we'll try moving into individual
arch/ABI that the CI tests cover for now. This commit should
pass for the following:

- mips32
- mips64
- musl32
- musl64
- android32
- android64
- arm32
- aarch64
- x86
- x86_64

Then we can figure out the powerpc variants.  This also prevents
potential errors for sparc64 which is not covered by CI.
@berkowski
Copy link
Contributor Author

Ok, so at this point we seem to have workable solutions except for:

  • OpenBSD: Conflicting #defines between termios.h and rpcsvc/rex.h
  • PowerPC: Doesn't seem to have BOTHER constant; can just remove.
  • sparc64: Not testable. Could blindly implement defines from linux source. Probably better to just leave unimpl'd until testable.

@alexcrichton
Copy link
Member

Looks great to me! I'm fine r+'ing once CI passes

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 22, 2017

📌 Commit fb11c7a has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Feb 22, 2017

⌛ Testing commit fb11c7a with merge 4eb4be4...

bors added a commit that referenced this pull request Feb 22, 2017
Added baudrate constants.

Addresses #528, adding baudrate constants for `bsd` and `notbsd` flavors of `unix`.

Passes `libc-test` locally on ubuntu 14.04 LTS with the additional (uncommitted) entries into in `build.rs`:

```
cfg.skip_const( move |name| {
        match name {
            # ... snip ...
            "PTRACE_O_SUSPEND_SECCOMP" |
            "CLONE_NEWCGROUP" |
            "NETLINK_LIST_MEMBERSHIPS" |
            "NETLINK_LISTEN_ALL_NSID" |
            "NETLINK_CAP_ACK" |
            "PR_CAP_AMBIENT_CLEAR_ALL" |
            "PR_CAP_AMBIENT_LOWER" |
            "PR_CAP_AMBIENT_RAISE" |
            "PR_CAP_AMBIENT_IS_SET" |
            "PR_CAP_AMBIENT" |
            "PR_FP_MODE_FRE" |
            "PR_FP_MODE_FR" |
            "PR_GET_FP_MODE" |
            "PR_SET_FP_MODE" |
            "PR_MPX_DISABLE_MANAGEMENT" |
            "PR_MPX_ENABLE_MANAGEMENT" |
            "PR_GET_THP_DISABLE" |
            "PR_SET_THP_DISABLE" |
            "PR_SET_MM_MAP_SIZE" |
            "PR_GET_MM_MAP_SIZE" |
            "PR_SET_MM_MAP" |
            "NLM_F_DUMP_FILTERED" |
            "EPOLLEXCLUSIVE" => true,

            _ => false,
        }
    });
```
I'm assuming this is because I'm stuck using `linux-libc-dev:3.13.0-24.46` for the moment and those constants are defined in newer versions.
@bors
Copy link
Contributor

bors commented Feb 22, 2017

💔 Test failed - status-travis

@berkowski
Copy link
Contributor Author

Traveling over the next couple of days - I'll peck at the issues when I can

@kamalmarhubi
Copy link
Contributor

The powerpc errors are confusing:

bad CBAUDEX value at byte 3: rust: 0 (0x0) != c 16 (0x10)

while

#define CBAUDEX 0000000

https://github.com/torvalds/linux/blob/v4.9/arch/powerpc/include/uapi/asm/termbits.h#L139

@kamalmarhubi
Copy link
Contributor

The openbsd errors are also confusing:

bad B50 value at byte 0: rust: 50 (0x32) != c 1 (0x1)
bad B75 value at byte 0: rust: 75 (0x4b) != c 2 (0x2)

while

#define B50	50
#define B75	75

https://github.com/openbsd/src/blob/5271000b44abe23907b73bbb3aa38ddf4a0bce08/sys/sys/termios.h#L213-L214

(50 is 0x32 and 75 is 0x4b)

@semarie
Copy link
Contributor

semarie commented Feb 23, 2017

@kamalmarhubi for openbsd the reason is values from termios.h are overrided later by rpcsvc/rex.h inclusion. (and due to #ifdef _SYS_TERMIOS_H_ / #define _SYS_TERMIOS_H_ the reinclusion didn't reset values).

for powerpc, it could be the same problem (I don't check source code deeper).

@berkowski
Copy link
Contributor Author

@semarie: I tried including termios.h after rpcsvc/rex.h in build.rs in e393a2d but it didn't seem to do the trick. Did I add the include in the correct place you intended?

The PPC stuff is interesting. I found https://lists.ozlabs.org/pipermail/linuxppc-dev/1999-January/000634.html which looks similar. Maybe this is an issue with qemu-ppc using old header files?

@semarie
Copy link
Contributor

semarie commented Feb 23, 2017

@berkowski yes it is where I intended but it isn't enough (I didn't think about that before).

  • termios.h is included - at line 100.
  • #define baudrate values are defined, and _SYS_TERMIOS_H_ header guard is also defined
  • when rpcsvc/rex.h is included, it redefines baudrate values
  • when termios.h is included a second time, due to _SYS_TERMIOS_H_ header guard, baudrate values aren't redefined.

@berkowski
Copy link
Contributor Author

berkowski commented Feb 23, 2017

@semarie Ah, ok. In that case I'll revert the change to build.rs in a new commit.

Adding `termios.h` behind `rpcsvc/rex.h` does not solve the #define
clash since `rex.h` also defines the `_SYS_TERMIOS_H_` include guard.
```
export TARGET=x86_64-unknown-openbsd
export QEMU=openbsd.qcow2
sh ci/run-docker.sh $TARGET
```
Passes when run localy.
@semarie
Copy link
Contributor

semarie commented Feb 24, 2017

it seems a bit odd that the test still pass without rpcsvc/rex.h. Something should have changed and the header isn't required anymore. But it is a good thing for this PR.

@berkowski
Copy link
Contributor Author

I couldn't find anything in bsd/netbsdlike/openbsdlike/openbsd.rs that referenced the "rex" specific identifiers so I gave it a shot. Maybe it got pulled in by accident to support some of the termios related constants.

Anyway, seems to be good to go now.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 24, 2017

📌 Commit 0048148 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Feb 24, 2017

⌛ Testing commit 0048148 with merge dc1aa0e...

bors added a commit that referenced this pull request Feb 24, 2017
Added baudrate constants.

Addresses #528, adding baudrate constants for `bsd` and `notbsd` flavors of `unix`.

Passes `libc-test` locally on ubuntu 14.04 LTS with the additional (uncommitted) entries into in `build.rs`:

```
cfg.skip_const( move |name| {
        match name {
            # ... snip ...
            "PTRACE_O_SUSPEND_SECCOMP" |
            "CLONE_NEWCGROUP" |
            "NETLINK_LIST_MEMBERSHIPS" |
            "NETLINK_LISTEN_ALL_NSID" |
            "NETLINK_CAP_ACK" |
            "PR_CAP_AMBIENT_CLEAR_ALL" |
            "PR_CAP_AMBIENT_LOWER" |
            "PR_CAP_AMBIENT_RAISE" |
            "PR_CAP_AMBIENT_IS_SET" |
            "PR_CAP_AMBIENT" |
            "PR_FP_MODE_FRE" |
            "PR_FP_MODE_FR" |
            "PR_GET_FP_MODE" |
            "PR_SET_FP_MODE" |
            "PR_MPX_DISABLE_MANAGEMENT" |
            "PR_MPX_ENABLE_MANAGEMENT" |
            "PR_GET_THP_DISABLE" |
            "PR_SET_THP_DISABLE" |
            "PR_SET_MM_MAP_SIZE" |
            "PR_GET_MM_MAP_SIZE" |
            "PR_SET_MM_MAP" |
            "NLM_F_DUMP_FILTERED" |
            "EPOLLEXCLUSIVE" => true,

            _ => false,
        }
    });
```
I'm assuming this is because I'm stuck using `linux-libc-dev:3.13.0-24.46` for the moment and those constants are defined in newer versions.
@bors
Copy link
Contributor

bors commented Feb 24, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing dc1aa0e to master...

@bors bors merged commit 0048148 into rust-lang:master Feb 24, 2017
@berkowski berkowski deleted the baud_constants branch February 24, 2017 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants