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

std.os: add INVALID_SOCKET #12081

Merged
merged 1 commit into from
Aug 24, 2022
Merged

std.os: add INVALID_SOCKET #12081

merged 1 commit into from
Aug 24, 2022

Conversation

yyny
Copy link
Contributor

@yyny yyny commented Jul 11, 2022

This constant returns an invalid socket_t that can be used as a sentinel value.

This constant returns an invalid `socket_t` that can be used as a sentinel
value.
@daurnimator
Copy link
Contributor

daurnimator commented Jul 12, 2022

I don't think this belongs here. -1 is not universal for other operating systems.
Also consider that in zig we frequently store file descriptors in unsigned integers.

IMO the name/concept really is windows specific, and hence should continue to live only there.

@yyny
Copy link
Contributor Author

yyny commented Jul 13, 2022

-1 is not universal for other operating systems.

First of, I don't think this is even true, could you provide a counterexample? Second, any hypothetical socket implementations aside, Zig itself only defines a socket implementation for POSIX systems (And for Windows), and POSIX requires -1 to be an invalid file descriptor (Any negative number, actually, as well as any value above OPEN_MAX, -1 is just the value that is typically returned). Using -1 will be perfectly fine for the foreseeable future.
Moving the definition to the system files (like os/linux.zig) might be a good idea, though.

Also consider that in Zig we frequently store file descriptors in unsigned integers.

Storing INVALID_SOCKET in an unsigned integer would be user error, it also contradicts your earlier concern, if all file descriptors can universally be assumed to be unsigned, then -1 is by definition an invalid file descriptor value. It won't even compile anyway, since INVALID_SOCKET has type socket_t, which is signed. And even if it was unsigned, a different value could be chosen. I don't think this concern is relevant.

IMO the name/concept really is windows specific, and hence should continue to live only there.

The concept is not Windows specific, POSIX also defines what constitutes an invalid socket/file descriptor value, and as mentioned above it is the only other socket implementation Zig currently supports.
This is exactly why I would like this change, the concept is universal, I need a value for it, and could use a portable constant for this in the standard library so I don't have to define it myself.

@kprotty
Copy link
Member

kprotty commented Jul 22, 2022

Are there any use cases of -1 in the stdlib that could be replaced by INVALID_SOCKET before it gets merged?

@arBmind
Copy link
Contributor

arBmind commented Jul 26, 2022

@kprotty Are there any use cases of -1 in the stdlib that could be replaced by INVALID_SOCKET before it gets merged?

After a check of all relevant zig files in the std library, I found no use of -1 to initialize a socket_t.

We have some places where we use -1 to initialize a fd_t, that someone might want to inspect deeper (possibly in a different issue)

Maybe io_uring.zig is using the fd_t because for linux it's the same as socket_t? I have not enough experience with io_uring to decide this.

@kprotty kprotty merged commit 0f01e81 into ziglang:master Aug 24, 2022
andrewrk added a commit that referenced this pull request Aug 24, 2022
This reverts commit 0f01e81.

This does not belong in `std.posix`, and it is already provided at
`std.os.windows.ws2_32.INVALID_SOCKET`.

See related issue #5019.
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.

4 participants