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

Port Native libraries to SunOS #34867

Merged
merged 1 commit into from
Apr 14, 2020
Merged

Port Native libraries to SunOS #34867

merged 1 commit into from
Apr 14, 2020

Conversation

am11
Copy link
Member

@am11 am11 commented Apr 12, 2020

Summary of changes:

  • Add Solaris filesystems in MountPoints.FormatInfo.
  • Set supported linker options.
  • Remove negative index from __c_static_assert__.
    • possesses ub per C spec and GCC errors out.
  • Separate i/o calls to set speed, as cfsetspeed is not supported.
  • Set terminal initial flags manually, as cfmakeraw is not supported.
  • Introspection for netpacket/packet.h and net/if_arp.h headers.
    • preferred feature detection to avoid further platform detection.
  • Add fopen fallback, as getmntinfo is not supported.
  • Add introspection for non legacy statfs(2) prototype.
    • Only legacy statfs(2) with different prototype is supported, preferred way is to use libc's statvfs(3).
  • Some RLIMIT_* resources are different/non-supported, mapped them accordingly.
  • Add fallback for getgrouplist(3).

Contributes to: #4173.

@jkotas jkotas requested a review from wfurt April 12, 2020 16:35
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.

cfmakeraw(&term);
#else
term.c_iflag &= ~(IMAXBEL|IGNBRK|BRKINT|PARMRK|ISTRIP|INLCR|IGNCR|ICRNL|IXON);
Copy link
Member

Choose a reason for hiding this comment

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

On Linux, cfmakeraw shows without the IMAXBEL. It was really just convenience and I'm wondering if we can get away without the fork and always list all options. (that can be done later)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I think three things can be simplified:

  • cfmakeraw -> list of options
  • cfsetspeed -> i/o speed separately
  • statfs -> statvfs
    • former is kernel call, latter is libc standard function, and Solaris/Illumos is known to err to the libc functions generally, without sysctl(2) like Linux or sysctl(3) like BSD.

I wanted to keep changes simple for the first pass, however, can make these changes and simplify the PR. Wdyt, should we make these three now or postpone it until later?

Copy link
Member

Choose a reason for hiding this comment

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

I would do it later. cc: @krwq

@@ -495,7 +511,11 @@ int32_t SystemIoPortsNative_TermiosReset(intptr_t handle, int32_t speed, int32_t
return -1;
}

#if HAVE_CFSETSPEED
Copy link
Member

Choose a reason for hiding this comment

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

same as above. cfsetspeed() is a 4.4BSD extension. We may use cfsetispeed and cfsetospeed on all platforms.

@am11
Copy link
Member Author

am11 commented Apr 13, 2020

I think this work item can have os-sunos and area-Infrastructure-libraries labels.

@ghost
Copy link

ghost commented Apr 13, 2020

Tagging @safern, @ViktorHofer as an area owner. If you would like to be tagged for a label, please notify danmosemsft.

@jkotas jkotas merged commit b88be15 into dotnet:master Apr 14, 2020
@am11 am11 deleted the feature/solaris/libraries-port branch April 14, 2020 01:26
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants