Skip to content

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.

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

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