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.

Sign up for GitHub

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

Jump to bottom

Fix newlib #3345

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Fix newlib #3345

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c1371d1
fix newlib definitions
zetanumbers Sep 8, 2023
1279b4d
remove EAI_MAX for newlib
zetanumbers Sep 8, 2023
829242f
Isolate horizon newlib definitions
zetanumbers Sep 8, 2023
6090661
WIP fixes (may not work)
zetanumbers Jan 4, 2024
1aa247c
WIP newlib fixes pt.2 (may not work)
zetanumbers Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 0 additions & 10 deletions src/unix/newlib/horizon/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,6 @@ pub const POLLERR: ::c_short = 0x0008;
pub const POLLHUP: ::c_short = 0x0010;
pub const POLLNVAL: ::c_short = 0x0020;

pub const EAI_AGAIN: ::c_int = 2;
pub const EAI_BADFLAGS: ::c_int = 3;
pub const EAI_FAIL: ::c_int = 4;
pub const EAI_SERVICE: ::c_int = 9;
pub const EAI_SYSTEM: ::c_int = 11;
pub const EAI_BADHINTS: ::c_int = 12;
pub const EAI_PROTOCOL: ::c_int = 13;
pub const EAI_OVERFLOW: ::c_int = 14;
pub const EAI_MAX: ::c_int = 15;

pub const AF_UNIX: ::c_int = 1;
pub const AF_INET6: ::c_int = 23;

Expand Down
105 changes: 73 additions & 32 deletions src/unix/newlib/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,58 @@ cfg_if! {
}

cfg_if! {
if #[cfg(any(target_os = "espidf"))] {
if #[cfg(target_os = "horizon")] {
pub type dev_t = u32;
pub type off_t = i64;
} else {
pub type dev_t = ::c_short;
pub type ino_t = ::c_ushort;
pub type off_t = ::c_long;
} else if #[cfg(any(target_os = "vita"))] {
pub type dev_t = ::c_short;
pub type ino_t = ::c_ushort;
pub type off_t = ::c_int;
} else {
pub type dev_t = u32;
}
}

cfg_if! {
if #[cfg(target_os = "horizon")] {
pub type ino_t = u32;
pub type off_t = i64;
} else if #[cfg(target_arch = "sparc")] {
pub type ino_t = ::c_ulong;
} else {
pub type ino_t = ::c_ushort;
}
}

pub type fsblkcnt_t = u64;
pub type fsfilcnt_t = u32;
pub type id_t = u32;
pub type key_t = ::c_int;

cfg_if! {
zetanumbers marked this conversation as resolved.
Show resolved Hide resolved
if #[cfg(target_os = "horizon")] {
pub type key_t = ::c_int;
} else {
pub type key_t = ::c_long;
}
}

pub type loff_t = ::c_longlong;
pub type mode_t = ::c_uint;
pub type nfds_t = u32;

cfg_if! {
if #[cfg(target_os = "horizon")] {
pub type mode_t = ::c_uint;
pub type nfds_t = u32;
} else {
pub type mode_t = u32;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto for mode_t and nfds_t?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mode_t:

https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/_types.h;h=d1112d5d102ca509331b9db5b27934b7cc403b32;hb=9e09d6ed83cce4777a5950412647ccc603040409#l90

DOS newlib isn't used or supported for rust by anyone, so i didn't that definition.
Couldn't figure out how to specify cfg attributes for SPARC definition due to __srv4__. However i haven't seen (even custom) rust target that would use that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nfds_t:

https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/sys/rtems/include/sys/poll.h;h=cc6ad49db033dfc63714d5062da30bf08b364dc3;hb=9e09d6ed83cce4777a5950412647ccc603040409#l42

Not sure about newlib structure of rtems, but i found it only there. Found it only there, doesn't look right, may revert.

pub type nfds_t = ::c_uint;
}
}

pub type nlink_t = ::c_ushort;
pub type pthread_t = ::c_ulong;

cfg_if! {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto for pthread_t?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/_pthreadtypes.h;h=75e9e1cbf71070e6ffef02ced33d0cd94ace0c14;hb=9e09d6ed83cce4777a5950412647ccc603040409#l32

__XMK__ seems to be not defined for rust's cfg

if #[cfg(target_os = "horizon")] {
pub type pthread_t = ::c_ulong;
} else {
pub type pthread_t = u32;
}
}
pub type pthread_key_t = ::c_uint;
pub type rlim_t = u32;

Expand Down Expand Up @@ -93,16 +121,16 @@ s! {
}

pub struct in_addr {
pub s_addr: ::in_addr_t,
pub s_addr: ::in_addr_t,
}

pub struct hostent {
pub h_name: *mut ::c_char,
pub h_aliases: *mut *mut ::c_char,
pub h_addrtype: ::c_int,
pub h_length: ::c_int,
pub h_addr_list: *mut *mut ::c_char,
pub h_addr: *mut ::c_char,
pub h_name: *mut ::c_char,
pub h_aliases: *mut *mut ::c_char,
pub h_addrtype: ::c_int,
pub h_length: ::c_int,
pub h_addr_list: *mut *mut ::c_char,
pub h_addr: *mut ::c_char,
}

pub struct pollfd {
Expand Down Expand Up @@ -546,8 +574,8 @@ pub const IFF_LINK2: ::c_int = 0x4000; // per link layer defined bit
pub const IFF_ALTPHYS: ::c_int = IFF_LINK2; // use alternate physical connection
pub const IFF_MULTICAST: ::c_int = 0x8000; // supports multicast

pub const TCP_NODELAY: ::c_int = 8193;
pub const TCP_MAXSEG: ::c_int = 8194;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These definitions must be included via a cfg_if! for horizonOS.

pub const TCP_NODELAY: ::c_int = 1;
zetanumbers marked this conversation as resolved.
Show resolved Hide resolved
pub const TCP_MAXSEG: ::c_int = 2;
pub const TCP_NOPUSH: ::c_int = 4;
pub const TCP_NOOPT: ::c_int = 8;
pub const TCP_KEEPIDLE: ::c_int = 256;
Expand All @@ -557,16 +585,21 @@ pub const TCP_KEEPCNT: ::c_int = 1024;
cfg_if! {
if #[cfg(target_os = "horizon")] {
pub const IP_TOS: ::c_int = 7;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these have wrong values for ESP-IDF, but I must say this is not due to this PR. It was also wrong before, and needs fixing:

pub const IP_TTL: ::c_int = 8;
pub const IP_MULTICAST_LOOP: ::c_int = 9;
pub const IP_MULTICAST_TTL: ::c_int = 10;
pub const IP_ADD_MEMBERSHIP: ::c_int = 11;
pub const IP_DROP_MEMBERSHIP: ::c_int = 12;
} else {
pub const IP_TOS: ::c_int = 3;
pub const IP_TTL: ::c_int = 4;
pub const IP_MULTICAST_IF: ::c_int = 9;
pub const IP_MULTICAST_TTL: ::c_int = 10;
pub const IP_MULTICAST_LOOP: ::c_int = 11;
pub const IP_ADD_MEMBERSHIP: ::c_int = 12;
pub const IP_DROP_MEMBERSHIP: ::c_int = 13;
}
}
pub const IP_TTL: ::c_int = 8;
pub const IP_MULTICAST_IF: ::c_int = 9;
pub const IP_MULTICAST_TTL: ::c_int = 10;
pub const IP_MULTICAST_LOOP: ::c_int = 11;
pub const IP_ADD_MEMBERSHIP: ::c_int = 11;
pub const IP_DROP_MEMBERSHIP: ::c_int = 12;

pub const IPV6_UNICAST_HOPS: ::c_int = 4;
pub const IPV6_MULTICAST_IF: ::c_int = 9;
Expand Down Expand Up @@ -598,10 +631,18 @@ pub const NI_NAMEREQD: ::c_int = 4;
pub const NI_NUMERICSERV: ::c_int = 0;
pub const NI_DGRAM: ::c_int = 0;

pub const EAI_FAMILY: ::c_int = -303;
pub const EAI_MEMORY: ::c_int = -304;
pub const EAI_NONAME: ::c_int = -305;
pub const EAI_SOCKTYPE: ::c_int = -307;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These definitions must be included via a cfg_if! for horizonOS. All other EAI definitions can be kept unified for all platforms.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. This was from past commit, so i haven't yet tried to preserve these mixed newlib and libctru values. You confirmed for me that this is the right approach. Now i can confidently finish this PR. Thank you.

pub const EAI_AGAIN: ::c_int = 2;
zetanumbers marked this conversation as resolved.
Show resolved Hide resolved
pub const EAI_BADFLAGS: ::c_int = 3;
pub const EAI_FAIL: ::c_int = 4;
pub const EAI_FAMILY: ::c_int = 5;
pub const EAI_MEMORY: ::c_int = 6;
pub const EAI_NONAME: ::c_int = 8;
pub const EAI_SERVICE: ::c_int = 9;
pub const EAI_SOCKTYPE: ::c_int = 10;
pub const EAI_SYSTEM: ::c_int = 11;
pub const EAI_BADHINTS: ::c_int = 12;
pub const EAI_PROTOCOL: ::c_int = 13;
pub const EAI_OVERFLOW: ::c_int = 14;

pub const EXIT_SUCCESS: ::c_int = 0;
pub const EXIT_FAILURE: ::c_int = 1;
Expand Down