Skip to content

Fcntl extension #196

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

Closed
wants to merge 13 commits into from
Closed

Fcntl extension #196

wants to merge 13 commits into from

Conversation

billabt
Copy link

@billabt billabt commented Dec 4, 2015

This pull request exposes the variadic fcntl() API to Swift for both OS X and Linux. It adds 3 different forms of the fcntl() API to the Swift stdlib. Tested on both OS X and Linux (Ubuntu 15.10).

@gribozavr
Copy link
Contributor

Since this is a change to the library API, so it should follow the Swift Evolution Process. Please write an informal proposal and send it to [email protected].

For the PR itself, please clang-format the C sources, and add tests for the new APIs.

@gribozavr gribozavr added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Dec 4, 2015
@emish
Copy link
Contributor

emish commented Dec 4, 2015

This document provides a thorough guide on submitting a proposal: https://github.com/apple/swift-evolution/blob/master/process.md

Bill Abt added 7 commits December 4, 2015 23:17

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
… functions.
…ay. Added tests for same. Fixed a few bug in the tests.
…reas on Apple OS's it's -1. Had logic reversed in new fcntl() expectXXXX() functions.
…onal to conform with other "C" library APIs.
// Change modes on existing file.
POSIXTests.test("fcntl F_GETFL/F_SETFL success with file") {
// Create and open file.
system("touch \(fn)")
Copy link
Contributor

Choose a reason for hiding this comment

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

system() won't work on iOS, you need to create file manually.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, will make the change...

On Dec 10, 2015, at 1:40 PM, Dmitri Gribenko [email protected] wrote:

In test/1_stdlib/POSIX.swift #196 (comment):

@@ -78,6 +81,112 @@ POSIXSemaphore.test("sem_open existing O_EXCL fail") {
let res2 = sem_unlink(semaphoreName)
expectEqual(0, res2)
}

+// Fail because file doesn't exist.
+POSIXTests.test("fcntl fail") {

  • let fd = open(strdup(fn), 0)
  • expectEqual(-1, fd)
  • expectEqual(ENOENT, errno)
  • let _ = fcntl(fd, F_GETFL)
  • expectEqual(EBADF, errno)
    +}

+// Change modes on existing file.
+POSIXTests.test("fcntl F_GETFL/F_SETFL success with file") {

  • // Create and open file.
  • system("touch (fn)")
    system() won't work on iOS, you need to create file manually.


Reply to this email directly or view it on GitHub https://github.com/apple/swift/pull/196/files#r47265320.

…he strdup() call that left over from earlier debugging.
@billabt
Copy link
Author

billabt commented Dec 10, 2015

All recommended changes complete and tested.

}

extern sem_t *_swift_Glibc_sem_open4(const char *name, int oflag,
mode_t mode, unsigned int value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation seems to be off.

@gribozavr
Copy link
Contributor

Could you squash everything into one commit and resolve conflicts against master?

@billabt
Copy link
Author

billabt commented Dec 10, 2015

Sure. I'll close this and re-issue a new pull request with a single commit plus the merge as soon as I get it done.

@billabt billabt closed this Dec 10, 2015
@kud1ing
Copy link

kud1ing commented Dec 11, 2015

@billabt A new PR is not strictly necessary: https://help.github.com/articles/about-git-rebase/
That being said, i often get lost while rebasing. :(

@billabt
Copy link
Author

billabt commented Dec 11, 2015

@kud1ing I've experienced the same thing. I find it safe to do it this way. Anyway, I resubmitted the PR here --> #413

Should be good to go.

slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018
build: add a cmake based build system
slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018
build: add a cmake based build system

Signed-off-by: Daniel A. Steffen <[email protected]>
dabelknap pushed a commit to dabelknap/swift that referenced this pull request Jan 17, 2019
Fix spacing for binary operator overloads
maldahleh pushed a commit to maldahleh/swift that referenced this pull request Oct 26, 2020
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
Remove realm configuration, doesn't work with sandbox
@AnthonyLatsis AnthonyLatsis removed the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Mar 22, 2023
DougGregor pushed a commit to DougGregor/swift that referenced this pull request Apr 28, 2024
Restructure the packaging to be more homogeneous and concise.

  - Swift Software Development Kit
    * Swift Compiler Tools
    * Swift Developer Tools
    * Swift Windows Runtime (AMD64|ARM64|x86)
    * Swift Windows SDK (AMD64|ARM64|x86)

This gives a more concise description that is also easier to understand
and map.  This sets up the path for further refinement of the installer
to provide more control over the installed components.

We do not suffix the Software Development Kit nor Compiler and Developer
Tools as there will be a singular version that matches the host that is
installed.  Therefore, there is no value in differentiating that.
However, multiple SDKs and Runtimes may be present, so identify the
architecture for them.
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

5 participants