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

Allow eventfd to be zero #62

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Allow eventfd to be zero #62

merged 1 commit into from
Feb 14, 2024

Conversation

bsbernd
Copy link

@bsbernd bsbernd commented Feb 12, 2024

Result of eventfd() might be zero - this is a valid file descriptor.

Also always return -errno (and 0) from ublksrv_setup_eventfd() and use it for the error message.

Result of eventfd() might be zero - this is a valid file
descriptor.

Also always return -errno (and 0) from ublksrv_setup_eventfd()
and use it for the error message.
@bsbernd
Copy link
Author

bsbernd commented Feb 12, 2024

@ming1 Just wanted to check how to use polling on the ring fd and noticed that q->efd was assumed to be never 0.

@ming1
Copy link
Collaborator

ming1 commented Feb 13, 2024

Result of eventfd() might be zero - this is a valid file descriptor.

file descriptor 0 is reserved for standard input, so it shouldn't be
returned from eventfd().

Do you see any problem with current way of treating zero
as failure?

Thanks,

@bsbernd
Copy link
Author

bsbernd commented Feb 13, 2024

I was just looking something up in the code, without using it. It is ensured that stdin is still open when eventfd is set up, in all cases? Just very unusual to treat fd=0 as special...

@ming1
Copy link
Collaborator

ming1 commented Feb 14, 2024

I was just looking something up in the code, without using it. It is ensured that stdin is still open when eventfd is set up, in all cases? Just very unusual to treat fd=0 as special...

Fair enough, merged.

thanks,

@ming1 ming1 merged commit d98ac75 into ublk-org:master Feb 14, 2024
2 of 3 checks passed
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