Skip to content

Implement __zipos_dup #972

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

Merged
merged 2 commits into from
Dec 1, 2023
Merged

Implement __zipos_dup #972

merged 2 commits into from
Dec 1, 2023

Conversation

mrdomino
Copy link
Collaborator

@mrdomino mrdomino commented Nov 30, 2023

Makes ZiposHandle reference-counted by an rc field in a union with its
freelist next pointer. The functions __zipos_free and __zipos_keep
function as incref/decref for it. Adds __zipos_postdup to fix metadata
on file descriptors after dup-like operations, and adds zipos support to
sys_dup_nt + sys_close_nt.

@mrdomino mrdomino marked this pull request as draft November 30, 2023 19:23
@mrdomino mrdomino force-pushed the zipos-dup branch 5 times, most recently from ed1579c to 7b4a032 Compare November 30, 2023 23:31
Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

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

Looking good so far. Thanks for the early glance.

@mrdomino mrdomino marked this pull request as ready for review December 1, 2023 00:17
@mrdomino mrdomino force-pushed the zipos-dup branch 3 times, most recently from 90bed99 to b961f4d Compare December 1, 2023 04:30
Makes ZiposHandle reference-counted by an `rc` field in a union with its
freelist `next` pointer. The functions `__zipos_free` and `__zipos_keep`
function as incref/decref for it. Adds `__zipos_postdup` to fix metadata
on file descriptors after dup-like operations, and adds zipos support to
`sys_dup_nt` + `sys_close_nt`.
rc is never a zipos file because it is always a previously unused file
descriptor. fd is never a zipos file because that case has been handled
above by __zipos_fcntl.
Copy link
Owner

@jart jart left a comment

Choose a reason for hiding this comment

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

Looks good to me. This is an impressive change. I cloned your branch locally and ran it on my test fleet of VMs (freebsd rhel7 xnu win10 openbsd netbsd pi silicon) using runit and runitd. I was amazed to see it all green.

I've contacted you about the copyright thing. Once that happens, I'll merge this.

@jart jart merged commit d1a745c into jart:master Dec 1, 2023
@mrdomino
Copy link
Collaborator Author

mrdomino commented Dec 1, 2023

Sadly the commit message fell out of sync with the actual changes, and I had a couple more changes in flight (implementing atomic reads and seeks even though POSIX says they don't exist because why not). What's the best approach - put the new changes on a new branch or force-push to main?

@jart
Copy link
Owner

jart commented Dec 1, 2023

It's probably too late to force push at this point. I'll make a mental note of it for the release notes.

@mrdomino mrdomino deleted the zipos-dup branch December 1, 2023 15:11
G4Vi pushed a commit to G4Vi/cosmopolitan that referenced this pull request Jan 19, 2024
* Implement __zipos_dup

Makes ZiposHandle reference-counted by an `rc` field in a union with its
freelist `next` pointer. The functions `__zipos_free` and `__zipos_keep`
function as incref/decref for it. Adds `__zipos_postdup` to fix metadata
on file descriptors after dup-like operations, and adds zipos support to
`sys_dup_nt` + `sys_close_nt`.

* Remove noop __zipos_postdup

rc is never a zipos file because it is always a previously unused file
descriptor. fd is never a zipos file because that case has been handled
above by __zipos_fcntl.
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.

2 participants